From f51792ae5e8422f58b2c64b2966707113a344bc1 Mon Sep 17 00:00:00 2001 From: Reed Allman Date: Sat, 23 Dec 2017 09:57:36 -0600 Subject: [PATCH] Timestamps on apps / routes (#614) * route updated_at * add app created at, fix some route updated_at bugs * add app updated_at TODO need to add tests through front end TODO for validation we don't really want to use the validate wrapper since it's a programmer error and not a user error, hopefully tests block this. * add tests for timestamps to exist / change on apps&routes * route equals at done, fix tests wit dis * fix up the equals sugar * add swagger * fix rebase * precisely allocate maps in clone * vetted * meh * fix api tests --- api/agent/protocol/json_test.go | 3 +- api/datastore/internal/datastoretest/test.go | 21 ++- api/datastore/mock.go | 2 +- .../4_add_route_updated_at.down.sql | 1 + .../migrations/4_add_route_updated_at.up.sql | 1 + .../migrations/5_add_app_created_at.down.sql | 1 + .../migrations/5_add_app_created_at.up.sql | 1 + .../migrations/6_add_app_updated_at.down.sql | 1 + .../migrations/6_add_app_updated_at.up.sql | 1 + api/datastore/sql/migrations/migrations.go | 144 +++++++++++++++++- api/datastore/sql/sql.go | 48 ++++-- api/models/app.go | 65 ++++++-- api/models/config.go | 31 ++++ api/models/route.go | 69 ++++++++- api/server/apps_create.go | 12 +- api/server/apps_test.go | 48 ++++++ api/server/routes_create_update.go | 3 - api/server/routes_test.go | 41 +++++ docs/swagger.yml | 20 +++ images/dind/build.sh | 2 +- images/error/build.sh | 2 +- images/fn-test-utils/build.sh | 2 +- test/fn-api-tests/routes_test.go | 5 +- 23 files changed, 459 insertions(+), 65 deletions(-) create mode 100644 api/datastore/sql/migrations/4_add_route_updated_at.down.sql create mode 100644 api/datastore/sql/migrations/4_add_route_updated_at.up.sql create mode 100644 api/datastore/sql/migrations/5_add_app_created_at.down.sql create mode 100644 api/datastore/sql/migrations/5_add_app_created_at.up.sql create mode 100644 api/datastore/sql/migrations/6_add_app_updated_at.down.sql create mode 100644 api/datastore/sql/migrations/6_add_app_updated_at.up.sql diff --git a/api/agent/protocol/json_test.go b/api/agent/protocol/json_test.go index 1a8764089..cad6a0429 100644 --- a/api/agent/protocol/json_test.go +++ b/api/agent/protocol/json_test.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "net/http" "net/url" - "reflect" "testing" "github.com/fnproject/fn/api/models" @@ -113,7 +112,7 @@ func TestJSONProtocolwriteJSONInputRequestWithoutData(t *testing.T) { t.Errorf("Request body assertion mismatch: expected: %s, got %s", "", incomingReq.Body) } - if ok := reflect.DeepEqual(req.Header, incomingReq.Protocol.Headers); !ok { + if !models.Headers(req.Header).Equals(models.Headers(incomingReq.Protocol.Headers)) { t.Errorf("Request headers assertion mismatch: expected: %s, got %s", req.Header, incomingReq.Protocol.Headers) } diff --git a/api/datastore/internal/datastoretest/test.go b/api/datastore/internal/datastoretest/test.go index 247f87c8b..efaca1b44 100644 --- a/api/datastore/internal/datastoretest/test.go +++ b/api/datastore/internal/datastoretest/test.go @@ -4,7 +4,6 @@ import ( "bytes" "context" "log" - "reflect" "testing" "time" @@ -289,7 +288,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if err != nil { t.Fatalf("Test InsertApp: error when storing new app: %s", err) } - if !reflect.DeepEqual(*inserted, *testApp) { + if !inserted.Equals(testApp) { t.Fatalf("Test InsertApp: expected to insert:\n%v\nbut got:\n%v", testApp, inserted) } @@ -300,13 +299,12 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { { // Set a config var - updated, err := ds.UpdateApp(ctx, - &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1"}}) + updated, err := ds.UpdateApp(ctx, &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1"}}) if err != nil { t.Fatalf("Test UpdateApp: error when updating app: %v", err) } expected := &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1"}} - if !reflect.DeepEqual(*updated, *expected) { + if !updated.Equals(expected) { t.Fatalf("Test UpdateApp: expected updated `%v` but got `%v`", expected, updated) } @@ -317,7 +315,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test UpdateApp: error when updating app: %v", err) } expected = &models.App{Name: testApp.Name, Config: map[string]string{"TEST": "1", "OTHER": "TEST"}} - if !reflect.DeepEqual(*updated, *expected) { + if !updated.Equals(expected) { t.Fatalf("Test UpdateApp: expected updated `%v` but got `%v`", expected, updated) } @@ -328,7 +326,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { t.Fatalf("Test UpdateApp: error when updating app: %v", err) } expected = &models.App{Name: testApp.Name, Config: map[string]string{"OTHER": "TEST"}} - if !reflect.DeepEqual(*updated, *expected) { + if !updated.Equals(expected) { t.Fatalf("Test UpdateApp: expected updated `%v` but got `%v`", expected, updated) } } @@ -496,9 +494,8 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { if err != nil { t.Fatalf("Test GetRoute: unexpected error %v", err) } - var expected models.Route = *testRoute - if !reflect.DeepEqual(*route, expected) { - t.Fatalf("Test InsertApp: expected to insert:\n%v\nbut got:\n%v", expected, *route) + if !route.Equals(testRoute) { + t.Fatalf("Test InsertApp: expected to insert:\n%v\nbut got:\n%v", testRoute, *route) } } @@ -545,7 +542,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { "Third": []string{"test", "test2"}, }, } - if !reflect.DeepEqual(*updated, *expected) { + if !updated.Equals(expected) { t.Fatalf("Test UpdateRoute: expected updated `%v` but got `%v`", expected, updated) } @@ -586,7 +583,7 @@ func Test(t *testing.T, dsf func(t *testing.T) models.Datastore) { "Third": []string{"test", "test2"}, }, } - if !reflect.DeepEqual(*updated, *expected) { + if !updated.Equals(expected) { t.Fatalf("Test UpdateRoute: expected updated:\n`%v`\nbut got:\n`%v`", expected, updated) } } diff --git a/api/datastore/mock.go b/api/datastore/mock.go index 44c713a99..8d7bfdf97 100644 --- a/api/datastore/mock.go +++ b/api/datastore/mock.go @@ -75,7 +75,7 @@ func (m *mock) UpdateApp(ctx context.Context, app *models.App) (*models.App, err if err != nil { return nil, err } - a.UpdateConfig(app.Config) + a.Update(app) return a.Clone(), nil } diff --git a/api/datastore/sql/migrations/4_add_route_updated_at.down.sql b/api/datastore/sql/migrations/4_add_route_updated_at.down.sql new file mode 100644 index 000000000..3b7031b52 --- /dev/null +++ b/api/datastore/sql/migrations/4_add_route_updated_at.down.sql @@ -0,0 +1 @@ +ALTER TABLE routes DROP COLUMN updated_at; diff --git a/api/datastore/sql/migrations/4_add_route_updated_at.up.sql b/api/datastore/sql/migrations/4_add_route_updated_at.up.sql new file mode 100644 index 000000000..f110016c4 --- /dev/null +++ b/api/datastore/sql/migrations/4_add_route_updated_at.up.sql @@ -0,0 +1 @@ +ALTER TABLE routes ADD updated_at varchar(256); diff --git a/api/datastore/sql/migrations/5_add_app_created_at.down.sql b/api/datastore/sql/migrations/5_add_app_created_at.down.sql new file mode 100644 index 000000000..0a50d859d --- /dev/null +++ b/api/datastore/sql/migrations/5_add_app_created_at.down.sql @@ -0,0 +1 @@ +ALTER TABLE apps DROP COLUMN created_at; diff --git a/api/datastore/sql/migrations/5_add_app_created_at.up.sql b/api/datastore/sql/migrations/5_add_app_created_at.up.sql new file mode 100644 index 000000000..38aea956f --- /dev/null +++ b/api/datastore/sql/migrations/5_add_app_created_at.up.sql @@ -0,0 +1 @@ +ALTER TABLE apps ADD created_at varchar(256); diff --git a/api/datastore/sql/migrations/6_add_app_updated_at.down.sql b/api/datastore/sql/migrations/6_add_app_updated_at.down.sql new file mode 100644 index 000000000..a59eaee2e --- /dev/null +++ b/api/datastore/sql/migrations/6_add_app_updated_at.down.sql @@ -0,0 +1 @@ +ALTER TABLE apps DROP COLUMN updated_at; diff --git a/api/datastore/sql/migrations/6_add_app_updated_at.up.sql b/api/datastore/sql/migrations/6_add_app_updated_at.up.sql new file mode 100644 index 000000000..5b8f715ce --- /dev/null +++ b/api/datastore/sql/migrations/6_add_app_updated_at.up.sql @@ -0,0 +1 @@ +ALTER TABLE apps ADD updated_at varchar(256); diff --git a/api/datastore/sql/migrations/migrations.go b/api/datastore/sql/migrations/migrations.go index 54af44440..c7a253e6b 100644 --- a/api/datastore/sql/migrations/migrations.go +++ b/api/datastore/sql/migrations/migrations.go @@ -6,6 +6,12 @@ // 2_add_call_stats.up.sql // 3_add_call_error.down.sql // 3_add_call_error.up.sql +// 4_add_route_updated_at.down.sql +// 4_add_route_updated_at.up.sql +// 5_add_app_created_at.down.sql +// 5_add_app_created_at.up.sql +// 6_add_app_updated_at.down.sql +// 6_add_app_updated_at.up.sql // DO NOT EDIT! package migrations @@ -108,7 +114,7 @@ func _1_add_route_created_atUpSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "1_add_route_created_at.up.sql", size: 40, mode: os.FileMode(420), modTime: time.Unix(1511259011, 0)} + info := bindataFileInfo{name: "1_add_route_created_at.up.sql", size: 40, mode: os.FileMode(420), modTime: time.Unix(1511919777, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -168,7 +174,7 @@ func _3_add_call_errorDownSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "3_add_call_error.down.sql", size: 37, mode: os.FileMode(420), modTime: time.Unix(1511265731, 0)} + info := bindataFileInfo{name: "3_add_call_error.down.sql", size: 37, mode: os.FileMode(420), modTime: time.Unix(1511301534, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -188,7 +194,127 @@ func _3_add_call_errorUpSql() (*asset, error) { return nil, err } - info := bindataFileInfo{name: "3_add_call_error.up.sql", size: 34, mode: os.FileMode(420), modTime: time.Unix(1511265909, 0)} + info := bindataFileInfo{name: "3_add_call_error.up.sql", size: 34, mode: os.FileMode(420), modTime: time.Unix(1511301534, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var __4_add_route_updated_atDownSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x28\xca\x2f\x2d\x49\x2d\x56\x70\x09\xf2\x0f\x50\x70\xf6\xf7\x09\xf5\xf5\x53\x28\x2d\x48\x49\x2c\x49\x4d\x89\x4f\x2c\xb1\xe6\x02\x04\x00\x00\xff\xff\xa4\x67\xb0\xea\x2b\x00\x00\x00") + +func _4_add_route_updated_atDownSqlBytes() ([]byte, error) { + return bindataRead( + __4_add_route_updated_atDownSql, + "4_add_route_updated_at.down.sql", + ) +} + +func _4_add_route_updated_atDownSql() (*asset, error) { + bytes, err := _4_add_route_updated_atDownSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "4_add_route_updated_at.down.sql", size: 43, mode: os.FileMode(420), modTime: time.Unix(1513728957, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var __4_add_route_updated_atUpSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x28\xca\x2f\x2d\x49\x2d\x56\x70\x74\x71\x51\x28\x2d\x48\x49\x2c\x49\x4d\x89\x4f\x2c\x51\x28\x4b\x2c\x4a\xce\x48\x2c\xd2\x30\x32\x35\xd3\xb4\xe6\x02\x04\x00\x00\xff\xff\x54\xf7\xac\x11\x30\x00\x00\x00") + +func _4_add_route_updated_atUpSqlBytes() ([]byte, error) { + return bindataRead( + __4_add_route_updated_atUpSql, + "4_add_route_updated_at.up.sql", + ) +} + +func _4_add_route_updated_atUpSql() (*asset, error) { + bytes, err := _4_add_route_updated_atUpSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "4_add_route_updated_at.up.sql", size: 48, mode: os.FileMode(420), modTime: time.Unix(1513730369, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var __5_add_app_created_atDownSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x48\x2c\x28\x28\x56\x70\x09\xf2\x0f\x50\x70\xf6\xf7\x09\xf5\xf5\x53\x48\x2e\x4a\x4d\x2c\x49\x4d\x89\x4f\x2c\xb1\xe6\x02\x04\x00\x00\xff\xff\xd2\xde\x5c\x98\x29\x00\x00\x00") + +func _5_add_app_created_atDownSqlBytes() ([]byte, error) { + return bindataRead( + __5_add_app_created_atDownSql, + "5_add_app_created_at.down.sql", + ) +} + +func _5_add_app_created_atDownSql() (*asset, error) { + bytes, err := _5_add_app_created_atDownSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "5_add_app_created_at.down.sql", size: 41, mode: os.FileMode(420), modTime: time.Unix(1513730497, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var __5_add_app_created_atUpSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x48\x2c\x28\x28\x56\x70\x74\x71\x51\x48\x2e\x4a\x4d\x2c\x49\x4d\x89\x4f\x2c\x51\x28\x4b\x2c\x4a\xce\x48\x2c\xd2\x30\x32\x35\xd3\xb4\xe6\x02\x04\x00\x00\xff\xff\x76\x6c\x0f\x45\x2e\x00\x00\x00") + +func _5_add_app_created_atUpSqlBytes() ([]byte, error) { + return bindataRead( + __5_add_app_created_atUpSql, + "5_add_app_created_at.up.sql", + ) +} + +func _5_add_app_created_atUpSql() (*asset, error) { + bytes, err := _5_add_app_created_atUpSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "5_add_app_created_at.up.sql", size: 46, mode: os.FileMode(420), modTime: time.Unix(1513730527, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var __6_add_app_updated_atDownSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x48\x2c\x28\x28\x56\x70\x09\xf2\x0f\x50\x70\xf6\xf7\x09\xf5\xf5\x53\x28\x2d\x48\x49\x2c\x49\x4d\x89\x4f\x2c\xb1\xe6\x02\x04\x00\x00\xff\xff\x31\x44\xd7\xcc\x29\x00\x00\x00") + +func _6_add_app_updated_atDownSqlBytes() ([]byte, error) { + return bindataRead( + __6_add_app_updated_atDownSql, + "6_add_app_updated_at.down.sql", + ) +} + +func _6_add_app_updated_atDownSql() (*asset, error) { + bytes, err := _6_add_app_updated_atDownSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "6_add_app_updated_at.down.sql", size: 41, mode: os.FileMode(420), modTime: time.Unix(1513733616, 0)} + a := &asset{bytes: bytes, info: info} + return a, nil +} + +var __6_add_app_updated_atUpSql = []byte("\x1f\x8b\x08\x00\x00\x00\x00\x00\x00\xff\x72\xf4\x09\x71\x0d\x52\x08\x71\x74\xf2\x71\x55\x48\x2c\x28\x28\x56\x70\x74\x71\x51\x28\x2d\x48\x49\x2c\x49\x4d\x89\x4f\x2c\x51\x28\x4b\x2c\x4a\xce\x48\x2c\xd2\x30\x32\x35\xd3\xb4\xe6\x02\x04\x00\x00\xff\xff\x65\x01\x8b\x34\x2e\x00\x00\x00") + +func _6_add_app_updated_atUpSqlBytes() ([]byte, error) { + return bindataRead( + __6_add_app_updated_atUpSql, + "6_add_app_updated_at.up.sql", + ) +} + +func _6_add_app_updated_atUpSql() (*asset, error) { + bytes, err := _6_add_app_updated_atUpSqlBytes() + if err != nil { + return nil, err + } + + info := bindataFileInfo{name: "6_add_app_updated_at.up.sql", size: 46, mode: os.FileMode(420), modTime: time.Unix(1513733621, 0)} a := &asset{bytes: bytes, info: info} return a, nil } @@ -251,6 +377,12 @@ var _bindata = map[string]func() (*asset, error){ "2_add_call_stats.up.sql": _2_add_call_statsUpSql, "3_add_call_error.down.sql": _3_add_call_errorDownSql, "3_add_call_error.up.sql": _3_add_call_errorUpSql, + "4_add_route_updated_at.down.sql": _4_add_route_updated_atDownSql, + "4_add_route_updated_at.up.sql": _4_add_route_updated_atUpSql, + "5_add_app_created_at.down.sql": _5_add_app_created_atDownSql, + "5_add_app_created_at.up.sql": _5_add_app_created_atUpSql, + "6_add_app_updated_at.down.sql": _6_add_app_updated_atDownSql, + "6_add_app_updated_at.up.sql": _6_add_app_updated_atUpSql, } // AssetDir returns the file names below a certain @@ -300,6 +432,12 @@ var _bintree = &bintree{nil, map[string]*bintree{ "2_add_call_stats.up.sql": &bintree{_2_add_call_statsUpSql, map[string]*bintree{}}, "3_add_call_error.down.sql": &bintree{_3_add_call_errorDownSql, map[string]*bintree{}}, "3_add_call_error.up.sql": &bintree{_3_add_call_errorUpSql, map[string]*bintree{}}, + "4_add_route_updated_at.down.sql": &bintree{_4_add_route_updated_atDownSql, map[string]*bintree{}}, + "4_add_route_updated_at.up.sql": &bintree{_4_add_route_updated_atUpSql, map[string]*bintree{}}, + "5_add_app_created_at.down.sql": &bintree{_5_add_app_created_atDownSql, map[string]*bintree{}}, + "5_add_app_created_at.up.sql": &bintree{_5_add_app_created_atUpSql, map[string]*bintree{}}, + "6_add_app_updated_at.down.sql": &bintree{_6_add_app_updated_atDownSql, map[string]*bintree{}}, + "6_add_app_updated_at.up.sql": &bintree{_6_add_app_updated_atUpSql, map[string]*bintree{}}, }} // RestoreAsset restores an asset under the given directory diff --git a/api/datastore/sql/sql.go b/api/datastore/sql/sql.go index 18ab81bf5..d1d55325e 100644 --- a/api/datastore/sql/sql.go +++ b/api/datastore/sql/sql.go @@ -37,6 +37,11 @@ import ( // // currently tested and working are postgres, mysql and sqlite3. +// TODO routes.created_at should be varchar(256), mysql will store 'text' +// fields not contiguous with other fields and this field is a fixed size, +// we'll get better locality with varchar. it's not terribly easy to do this +// with migrations (sadly, need complex transaction) + var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( app_name varchar(256) NOT NULL, path varchar(256) NOT NULL, @@ -49,12 +54,15 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( headers text NOT NULL, config text NOT NULL, created_at text, + updated_at varchar(256), PRIMARY KEY (app_name, path) );`, `CREATE TABLE IF NOT EXISTS apps ( name varchar(256) NOT NULL PRIMARY KEY, - config text NOT NULL + config text NOT NULL, + created_at varchar(256), + updated_at varchar(256) );`, `CREATE TABLE IF NOT EXISTS calls ( @@ -78,7 +86,7 @@ var tables = [...]string{`CREATE TABLE IF NOT EXISTS routes ( } const ( - routeSelector = `SELECT app_name, path, image, format, memory, type, timeout, idle_timeout, headers, config, created_at FROM routes` + routeSelector = `SELECT app_name, path, image, format, memory, type, timeout, idle_timeout, headers, config, created_at, updated_at FROM routes` callSelector = `SELECT id, created_at, started_at, completed_at, status, app_name, path, stats, error FROM calls` ) @@ -255,7 +263,18 @@ func (ds *sqlStore) clear() error { } func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App, error) { - query := ds.db.Rebind("INSERT INTO apps (name, config) VALUES (:name, :config);") + query := ds.db.Rebind(`INSERT INTO apps ( + name, + config, + created_at, + updated_at + ) + VALUES ( + :name, + :config, + :created_at, + :updated_at + );`) _, err := ds.db.NamedExecContext(ctx, query, app) if err != nil { switch err := err.(type) { @@ -281,7 +300,9 @@ func (ds *sqlStore) InsertApp(ctx context.Context, app *models.App) (*models.App func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models.App, error) { app := &models.App{Name: newapp.Name} err := ds.Tx(func(tx *sqlx.Tx) error { - query := tx.Rebind(`SELECT config FROM apps WHERE name=?`) + // NOTE: must query whole object since we're returning app, Update logic + // must only modify modifiable fields (as seen here). need to fix brittle.. + query := tx.Rebind(`SELECT name, config, created_at, updated_at FROM apps WHERE name=?`) row := tx.QueryRowxContext(ctx, query, app.Name) err := row.StructScan(app) @@ -291,9 +312,9 @@ func (ds *sqlStore) UpdateApp(ctx context.Context, newapp *models.App) (*models. return err } - app.UpdateConfig(newapp.Config) + app.Update(newapp) - query = tx.Rebind(`UPDATE apps SET config=:config WHERE name=:name`) + query = tx.Rebind(`UPDATE apps SET config=:config, updated_at=:updated_at WHERE name=:name`) res, err := tx.NamedExecContext(ctx, query, app) if err != nil { return err @@ -346,7 +367,7 @@ func (ds *sqlStore) RemoveApp(ctx context.Context, appName string) error { } func (ds *sqlStore) GetApp(ctx context.Context, name string) (*models.App, error) { - query := ds.db.Rebind(`SELECT name, config FROM apps WHERE name=?`) + query := ds.db.Rebind(`SELECT name, config, created_at, updated_at FROM apps WHERE name=?`) row := ds.db.QueryRowxContext(ctx, query, name) var res models.App @@ -369,10 +390,7 @@ func (ds *sqlStore) GetApps(ctx context.Context, filter *models.AppFilter) ([]*m if err != nil { return nil, err } - // fmt.Printf("QUERY: %v\n", query) - // fmt.Printf("ARGS: %v\n", args) - // hmm, should this have DISTINCT in it? shouldn't be possible to have two apps with same name - query = ds.db.Rebind(fmt.Sprintf("SELECT DISTINCT name, config FROM apps %s", query)) + query = ds.db.Rebind(fmt.Sprintf("SELECT DISTINCT name, config, created_at, updated_at FROM apps %s", query)) rows, err := ds.db.QueryxContext(ctx, query, args...) if err != nil { return nil, err @@ -427,7 +445,8 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode idle_timeout, headers, config, - created_at + created_at, + updated_at ) VALUES ( :app_name, @@ -440,7 +459,8 @@ func (ds *sqlStore) InsertRoute(ctx context.Context, route *models.Route) (*mode :idle_timeout, :headers, :config, - :created_at + :created_at, + :updated_at );`) _, err = tx.NamedExecContext(ctx, query, route) @@ -479,7 +499,7 @@ func (ds *sqlStore) UpdateRoute(ctx context.Context, newroute *models.Route) (*m idle_timeout = :idle_timeout, headers = :headers, config = :config, - created_at = :created_at + updated_at = :updated_at WHERE app_name=:app_name AND path=:path;`) res, err := tx.NamedExecContext(ctx, query, &route) diff --git a/api/models/app.go b/api/models/app.go index 475d76057..46dedd500 100644 --- a/api/models/app.go +++ b/api/models/app.go @@ -1,8 +1,29 @@ package models +import ( + "time" + + "github.com/go-openapi/strfmt" +) + type App struct { - Name string `json:"name" db:"name"` - Config Config `json:"config,omitempty" db:"config"` + Name string `json:"name" db:"name"` + Config Config `json:"config,omitempty" db:"config"` + CreatedAt strfmt.DateTime `json:"created_at,omitempty" db:"created_at"` + UpdatedAt strfmt.DateTime `json:"updated_at,omitempty" db:"updated_at"` +} + +func (a *App) SetDefaults() { + if time.Time(a.CreatedAt).IsZero() { + a.CreatedAt = strfmt.DateTime(time.Now()) + } + if time.Time(a.UpdatedAt).IsZero() { + a.UpdatedAt = strfmt.DateTime(time.Now()) + } + if a.Config == nil { + // keeps the json from being nil + a.Config = map[string]string{} + } } func (a *App) Validate() error { @@ -21,24 +42,42 @@ func (a *App) Validate() error { } func (a *App) Clone() *App { - var c App - c.Name = a.Name + clone := new(App) + *clone = *a // shallow copy + + // now deep copy the map if a.Config != nil { - c.Config = make(Config) + clone.Config = make(Config, len(a.Config)) for k, v := range a.Config { - c.Config[k] = v + clone.Config[k] = v } } - return &c + return clone } -// UpdateConfig adds entries from patch to a.Config, and removes entries with empty values. -func (a *App) UpdateConfig(patch Config) { - if patch != nil { +func (a1 *App) Equals(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.Name == a2.Name + eq = eq && a1.Config.Equals(a2.Config) + // 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 removes entries with empty values. +func (a *App) Update(src *App) { + original := a.Clone() + + if src.Config != nil { if a.Config == nil { a.Config = make(Config) } - for k, v := range patch { + for k, v := range src.Config { if v == "" { delete(a.Config, k) } else { @@ -46,6 +85,10 @@ func (a *App) UpdateConfig(patch Config) { } } } + + if !a.Equals(original) { + a.UpdatedAt = strfmt.DateTime(time.Now()) + } } // AppFilter is the filter used for querying apps diff --git a/api/models/config.go b/api/models/config.go index f3e0d75f7..67b36b360 100644 --- a/api/models/config.go +++ b/api/models/config.go @@ -14,6 +14,19 @@ func (c *Config) Validate() error { return nil } +func (c1 Config) Equals(c2 Config) bool { + if len(c1) != len(c2) { + return false + } + for k1, v1 := range c1 { + v2, _ := c2[k1] + if v1 != v2 { + return false + } + } + return true +} + // implements sql.Valuer, returning a string func (c Config) Value() (driver.Value, error) { if len(c) < 1 { @@ -56,6 +69,24 @@ func (c *Config) Scan(value interface{}) error { // Headers is an http.Header that implements additional methods. type Headers http.Header +func (h1 Headers) Equals(h2 Headers) bool { + if len(h1) != len(h2) { + return false + } + for k1, v1s := range h1 { + v2s, _ := h2[k1] + if len(v2s) != len(v1s) { + return false + } + for i, v1 := range v1s { + if v2s[i] != v1 { + return false + } + } + } + return true +} + // implements sql.Valuer, returning a string func (h Headers) Value() (driver.Value, error) { if len(h) < 1 { diff --git a/api/models/route.go b/api/models/route.go index bb871d185..c2a0ce7a3 100644 --- a/api/models/route.go +++ b/api/models/route.go @@ -5,6 +5,7 @@ import ( "net/url" "path" "strings" + "time" "github.com/go-openapi/strfmt" ) @@ -19,7 +20,7 @@ const ( MaxIdleTimeout = MaxAsyncTimeout ) -var RouteMaxMemory = uint64(8 * 1024) // 8GB TODO should probably be a var of machine max? +var RouteMaxMemory = uint64(8 * 1024) type Routes []*Route @@ -35,6 +36,7 @@ type Route struct { IdleTimeout int32 `json:"idle_timeout" db:"idle_timeout"` Config Config `json:"config,omitempty" db:"config"` CreatedAt strfmt.DateTime `json:"created_at,omitempty" db:"created_at"` + UpdatedAt strfmt.DateTime `json:"updated_at,omitempty" db:"updated_at"` } // SetDefaults sets zeroed field to defaults. @@ -56,6 +58,7 @@ func (r *Route) SetDefaults() { } if r.Config == nil { + // keeps the json from being nil r.Config = map[string]string{} } @@ -66,6 +69,14 @@ func (r *Route) SetDefaults() { if r.IdleTimeout == 0 { r.IdleTimeout = DefaultIdleTimeout } + + if time.Time(r.CreatedAt).IsZero() { + r.CreatedAt = strfmt.DateTime(time.Now()) + } + + if time.Time(r.UpdatedAt).IsZero() { + r.UpdatedAt = strfmt.DateTime(time.Now()) + } } // Validate validates all field values, returning the first error, if any. @@ -121,16 +132,54 @@ func (r *Route) Validate() error { } func (r *Route) Clone() *Route { - var clone Route - clone.AppName = r.AppName - clone.Path = r.Path - clone.Update(r) - return &clone + clone := new(Route) + *clone = *r // shallow copy + + // now deep copy the maps + if r.Config != nil { + clone.Config = make(Config, len(r.Config)) + for k, v := range r.Config { + clone.Config[k] = v + } + } + if r.Headers != nil { + clone.Headers = make(Headers, len(r.Headers)) + for k, v := range r.Headers { + // TODO technically, we need to deep copy this slice... + clone.Headers[k] = v + } + } + return clone } -// Update updates fields in r with non-zero field values from new. -// 0-length slice Header values, and empty-string Config values trigger removal of map entry. +func (r1 *Route) Equals(r2 *Route) 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 && r1.AppName == r2.AppName + eq = eq && r1.Path == r2.Path + eq = eq && r1.Image == r2.Image + eq = eq && r1.Memory == r2.Memory + eq = eq && r1.Headers.Equals(r2.Headers) + eq = eq && r1.Type == r2.Type + eq = eq && r1.Format == r2.Format + eq = eq && r1.Timeout == r2.Timeout + eq = eq && r1.IdleTimeout == r2.IdleTimeout + eq = eq && r1.Config.Equals(r2.Config) + // 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(r1.CreatedAt).Equal(time.Time(r2.CreatedAt)) + //eq = eq && time.Time(r2.UpdatedAt).Equal(time.Time(r2.UpdatedAt)) + return eq +} + +// Update updates fields in r 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. func (r *Route) Update(new *Route) { + original := r.Clone() + if new.Image != "" { r.Image = new.Image } @@ -173,6 +222,10 @@ func (r *Route) Update(new *Route) { } } } + + if !r.Equals(original) { + r.UpdatedAt = strfmt.DateTime(time.Now()) + } } type RouteFilter struct { diff --git a/api/server/apps_create.go b/api/server/apps_create.go index 97ffe5b44..97c2942df 100644 --- a/api/server/apps_create.go +++ b/api/server/apps_create.go @@ -18,29 +18,31 @@ func (s *Server) handleAppCreate(c *gin.Context) { return } - if wapp.App == nil { + app := wapp.App + if app == nil { handleErrorResponse(c, models.ErrAppsMissingNew) return } - if err = wapp.Validate(); err != nil { + app.SetDefaults() + if err = app.Validate(); err != nil { handleErrorResponse(c, err) return } - err = s.FireBeforeAppCreate(ctx, wapp.App) + err = s.FireBeforeAppCreate(ctx, app) if err != nil { handleErrorResponse(c, err) return } - app, err := s.datastore.InsertApp(ctx, wapp.App) + app, err = s.datastore.InsertApp(ctx, app) if err != nil { handleErrorResponse(c, err) return } - err = s.FireAfterAppCreate(ctx, wapp.App) + err = s.FireAfterAppCreate(ctx, app) if err != nil { handleErrorResponse(c, err) return diff --git a/api/server/apps_test.go b/api/server/apps_test.go index 3522711d9..75b311062 100644 --- a/api/server/apps_test.go +++ b/api/server/apps_test.go @@ -8,6 +8,7 @@ import ( "net/http" "strings" "testing" + "time" "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/logs" @@ -75,6 +76,28 @@ func TestAppCreate(t *testing.T) { i, test.expectedError.Error()) } } + + if test.expectedCode == http.StatusOK { + var awrap models.AppWrapper + err := json.NewDecoder(rec.Body).Decode(&awrap) + if err != nil { + t.Log(buf.String()) + t.Errorf("Test %d: error decoding body for 'ok' json, it was a lie: %v", i, err) + } + + app := awrap.App + + // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 + if time.Time(app.CreatedAt).Before(time.Now().Add(-1 * time.Hour)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected created_at to be set on app, it wasn't: %s", i, app.CreatedAt) + } + if !(time.Time(app.CreatedAt)).Equal(time.Time(app.UpdatedAt)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected updated_at to be set and same as created at, it wasn't: %s %s", i, app.CreatedAt, app.UpdatedAt) + } + } + cancel() } } @@ -290,6 +313,31 @@ func TestAppUpdate(t *testing.T) { } } + if test.expectedCode == http.StatusOK { + var awrap models.AppWrapper + err := json.NewDecoder(rec.Body).Decode(&awrap) + if err != nil { + t.Log(buf.String()) + t.Errorf("Test %d: error decoding body for 'ok' json, it was a lie: %v", i, err) + } + + app := awrap.App + // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 + if time.Time(app.UpdatedAt).Before(time.Now().Add(-1 * time.Hour)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected updated_at to be set on app, it wasn't: %s", i, app.UpdatedAt) + } + + // this isn't perfect, since a PATCH could succeed without updating any + // fields (among other reasons), but just don't make a test for that or + // special case (the body or smth) to ignore it here! + // this is a decent approximation that the timestamp gets changed + if (time.Time(app.UpdatedAt)).Equal(time.Time(app.CreatedAt)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected updated_at to not be the same as created at, it wasn't: %s %s", i, app.CreatedAt, app.UpdatedAt) + } + } + cancel() } } diff --git a/api/server/routes_create_update.go b/api/server/routes_create_update.go index 98cfbdc20..729253001 100644 --- a/api/server/routes_create_update.go +++ b/api/server/routes_create_update.go @@ -5,12 +5,10 @@ import ( "net/http" "path" "strings" - "time" "github.com/fnproject/fn/api" "github.com/fnproject/fn/api/models" "github.com/gin-gonic/gin" - "github.com/go-openapi/strfmt" ) /* handleRouteCreateOrUpdate is used to handle POST PUT and PATCH for routes. @@ -51,7 +49,6 @@ func (s *Server) handleRoutesPostPutPatch(c *gin.Context) { } func (s *Server) submitRoute(ctx context.Context, wroute *models.RouteWrapper) error { - wroute.Route.CreatedAt = strfmt.DateTime(time.Now()) wroute.Route.SetDefaults() err := wroute.Route.Validate() if err != nil { diff --git a/api/server/routes_test.go b/api/server/routes_test.go index e7205ec70..d278c78d2 100644 --- a/api/server/routes_test.go +++ b/api/server/routes_test.go @@ -7,6 +7,7 @@ import ( "net/http" "strings" "testing" + "time" "github.com/fnproject/fn/api/datastore" "github.com/fnproject/fn/api/logs" @@ -49,6 +50,46 @@ func (test *routeTestCase) run(t *testing.T, i int, buf *bytes.Buffer) { i, test.expectedError, resp.Error.Message) } } + + if test.expectedCode == http.StatusOK { + var rwrap models.RouteWrapper + err := json.NewDecoder(rec.Body).Decode(&rwrap) + if err != nil { + t.Log(buf.String()) + t.Errorf("Test %d: error decoding body for 'ok' json, it was a lie: %v", i, err) + } + + route := rwrap.Route + if test.method == http.MethodPost { + // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 + if time.Time(route.CreatedAt).Before(time.Now().Add(-1 * time.Hour)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected created_at to be set on route, it wasn't: %s", i, route.CreatedAt) + } + if !(time.Time(route.CreatedAt)).Equal(time.Time(route.UpdatedAt)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected updated_at to be set and same as created at, it wasn't: %s %s", i, route.CreatedAt, route.UpdatedAt) + } + } + + if test.method == http.MethodPatch { + // IsZero() doesn't really work, this ensures it's not unset as long as we're not in 1970 + if time.Time(route.UpdatedAt).Before(time.Now().Add(-1 * time.Hour)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected updated_at to be set on route, it wasn't: %s", i, route.UpdatedAt) + } + + // this isn't perfect, since a PATCH could succeed without updating any + // fields (among other reasons), but just don't make a test for that or + // special case (the body or smth) to ignore it here! + // this is a decent approximation that the timestamp gets changed + if (time.Time(route.UpdatedAt)).Equal(time.Time(route.CreatedAt)) { + t.Log(buf.String()) + t.Errorf("Test %d: expected updated_at to not be the same as created at, it wasn't: %s %s", i, route.CreatedAt, route.UpdatedAt) + } + } + } + cancel() buf.Reset() } diff --git a/docs/swagger.yml b/docs/swagger.yml index 360f32ded..de0315319 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -516,6 +516,16 @@ definitions: default: 30 format: int32 description: Hot functions idle timeout before termination. Value in Seconds + created_at: + type: string + format: date-time + description: Time when route was created. Always in UTC. + readOnly: true + updated_at: + type: string + format: date-time + description: Most recent time that route was updated. Always in UTC. + readOnly: true App: type: object @@ -529,6 +539,16 @@ definitions: description: Application configuration, applied to all routes. additionalProperties: type: string + created_at: + type: string + format: date-time + description: Time when app was created. Always in UTC. + readOnly: true + updated_at: + type: string + format: date-time + description: Most recent time that app was updated. Always in UTC. + readOnly: true Version: type: object diff --git a/images/dind/build.sh b/images/dind/build.sh index 9e497029d..a50ffce24 100755 --- a/images/dind/build.sh +++ b/images/dind/build.sh @@ -1,3 +1,3 @@ set -ex -docker build --build-arg HTTP_PROXY -t fnproject/dind:latest . +docker build --build-arg HTTPS_PROXY --build-arg HTTP_PROXY -t fnproject/dind:latest . diff --git a/images/error/build.sh b/images/error/build.sh index 06db1e371..d1ed85342 100755 --- a/images/error/build.sh +++ b/images/error/build.sh @@ -1,4 +1,4 @@ #!/bin/bash set -ex -docker build -t fnproject/error . +docker build --build-arg HTTPS_PROXY --build-arg HTTP_PROXY -t fnproject/error . diff --git a/images/fn-test-utils/build.sh b/images/fn-test-utils/build.sh index 66f91bee0..c6319b8fe 100755 --- a/images/fn-test-utils/build.sh +++ b/images/fn-test-utils/build.sh @@ -1,2 +1,2 @@ set -e -docker build -t fnproject/fn-test-utils:latest . +docker build --build-arg HTTPS_PROXY --build-arg HTTP_PROXY -t fnproject/fn-test-utils:latest . diff --git a/test/fn-api-tests/routes_test.go b/test/fn-api-tests/routes_test.go index 07815e844..1dece0808 100644 --- a/test/fn-api-tests/routes_test.go +++ b/test/fn-api-tests/routes_test.go @@ -1,12 +1,11 @@ package tests -// import ( "testing" "github.com/fnproject/fn/api/id" + api_models "github.com/fnproject/fn/api/models" "github.com/fnproject/fn_go/models" - "reflect" ) func TestRoutes(t *testing.T) { @@ -203,7 +202,7 @@ func TestRoutes(t *testing.T) { routeHeaders["B"] = []string{"b"} DeployRoute(t, s.Context, s.Client, s.AppName, s.RoutePath, s.Image, s.RouteType, s.Format, s.RouteConfig, routeHeaders) sameRoute := DeployRoute(t, s.Context, s.Client, s.AppName, s.RoutePath, s.Image, s.RouteType, s.Format, s.RouteConfig, routeHeaders) - if ok := reflect.DeepEqual(sameRoute.Headers, routeHeaders); !ok { + if !api_models.Headers(sameRoute.Headers).Equals(api_models.Headers(routeHeaders)) { t.Error("Route headers should remain the same after multiple deploys with exact the same parameters") } DeleteApp(t, s.Context, s.Client, s.AppName)