From 9382f0b1334b62e92d32f765a5808138e4dd617f Mon Sep 17 00:00:00 2001 From: Pedro Nasser Date: Wed, 7 Dec 2016 14:59:54 -0200 Subject: [PATCH] fix datastore Put and added tests (#402) --- api/datastore/bolt/bolt.go | 8 ++++ api/datastore/bolt_test.go | 63 ++++++++++++++++++------------ api/datastore/datastore_test.go | 31 +++++++++++++++ api/datastore/postgres/postgres.go | 10 ++++- api/datastore/postgres_test.go | 56 ++++++++++++++++++++------ api/models/datastore.go | 1 + 6 files changed, 132 insertions(+), 37 deletions(-) create mode 100644 api/datastore/datastore_test.go diff --git a/api/datastore/bolt/bolt.go b/api/datastore/bolt/bolt.go index ccb5df13e..5335bb71b 100644 --- a/api/datastore/bolt/bolt.go +++ b/api/datastore/bolt/bolt.go @@ -505,6 +505,10 @@ func (ds *BoltDatastore) GetRoutes(ctx context.Context, filter *models.RouteFilt } func (ds *BoltDatastore) Put(ctx context.Context, key, value []byte) error { + if key == nil || len(key) == 0 { + return models.ErrDatastoreEmptyKey + } + ds.db.Update(func(tx *bolt.Tx) error { b := tx.Bucket(ds.extrasBucket) // todo: maybe namespace by app? err := b.Put(key, value) @@ -514,6 +518,10 @@ func (ds *BoltDatastore) Put(ctx context.Context, key, value []byte) error { } func (ds *BoltDatastore) Get(ctx context.Context, key []byte) ([]byte, error) { + if key == nil || len(key) == 0 { + return nil, models.ErrDatastoreEmptyKey + } + var ret []byte ds.db.View(func(tx *bolt.Tx) error { b := tx.Bucket(ds.extrasBucket) diff --git a/api/datastore/bolt_test.go b/api/datastore/bolt_test.go index c5f443317..a33fa98b4 100644 --- a/api/datastore/bolt_test.go +++ b/api/datastore/bolt_test.go @@ -1,27 +1,13 @@ package datastore import ( - "bytes" "context" - "log" "os" "testing" - "github.com/Sirupsen/logrus" - "github.com/gin-gonic/gin" "github.com/iron-io/functions/api/models" ) -func setLogBuffer() *bytes.Buffer { - var buf bytes.Buffer - buf.WriteByte('\n') - logrus.SetOutput(&buf) - gin.DefaultErrorWriter = &buf - gin.DefaultWriter = &buf - log.SetOutput(&buf) - return &buf -} - const tmpBolt = "/tmp/func_test_bolt.db" func TestBolt(t *testing.T) { @@ -35,16 +21,6 @@ func TestBolt(t *testing.T) { t.Fatalf("Error when creating datastore: %v", err) } - testApp := &models.App{ - Name: "Test", - } - - testRoute := &models.Route{ - AppName: testApp.Name, - Path: "/test", - Image: "iron/hello", - } - // Testing insert app _, err = ds.InsertApp(ctx, nil) if err != models.ErrDatastoreEmptyApp { @@ -272,4 +248,43 @@ func TestBolt(t *testing.T) { t.Log(buf.String()) t.Fatalf("Test RemoveApp: failed to remove the route") } + + // Testing Put/Get + err = ds.Put(ctx, nil, nil) + if err != models.ErrDatastoreEmptyKey { + t.Log(buf.String()) + t.Fatalf("Test Put(nil,nil): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyKey, err) + } + + err = ds.Put(ctx, []byte("test"), []byte("success")) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + + val, err := ds.Get(ctx, []byte("test")) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + if string(val) != "success" { + t.Log(buf.String()) + t.Fatalf("Test Get: expected value to be `%v`, but it was `%v`", "success", string(val)) + } + + err = ds.Put(ctx, []byte("test"), nil) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + + val, err = ds.Get(ctx, []byte("test")) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + if string(val) != "" { + t.Log(buf.String()) + t.Fatalf("Test Get: expected value to be `%v`, but it was `%v`", "", string(val)) + } } diff --git a/api/datastore/datastore_test.go b/api/datastore/datastore_test.go new file mode 100644 index 000000000..c1ad34845 --- /dev/null +++ b/api/datastore/datastore_test.go @@ -0,0 +1,31 @@ +package datastore + +import ( + "bytes" + "github.com/Sirupsen/logrus" + "github.com/gin-gonic/gin" + "github.com/iron-io/functions/api/models" + "log" +) + +func setLogBuffer() *bytes.Buffer { + var buf bytes.Buffer + buf.WriteByte('\n') + logrus.SetOutput(&buf) + gin.DefaultErrorWriter = &buf + gin.DefaultWriter = &buf + log.SetOutput(&buf) + return &buf +} + +var testApp = &models.App{ + Name: "Test", +} + +var testRoute = &models.Route{ + AppName: testApp.Name, + Path: "/test", + Image: "iron/hello", + Type: "sync", + Format: "http", +} diff --git a/api/datastore/postgres/postgres.go b/api/datastore/postgres/postgres.go index de37206b2..e02ca0e9b 100644 --- a/api/datastore/postgres/postgres.go +++ b/api/datastore/postgres/postgres.go @@ -524,6 +524,10 @@ func buildFilterRouteQuery(filter *models.RouteFilter) string { } func (ds *PostgresDatastore) Put(ctx context.Context, key, value []byte) error { + if key == nil || len(key) == 0 { + return models.ErrDatastoreEmptyKey + } + _, err := ds.db.Exec(` INSERT INTO extras ( key, @@ -531,7 +535,7 @@ func (ds *PostgresDatastore) Put(ctx context.Context, key, value []byte) error { ) VALUES ($1, $2) ON CONFLICT (key) DO UPDATE SET - value = $1; + value = $2; `, string(key), string(value)) if err != nil { @@ -542,6 +546,10 @@ func (ds *PostgresDatastore) Put(ctx context.Context, key, value []byte) error { } func (ds *PostgresDatastore) Get(ctx context.Context, key []byte) ([]byte, error) { + if key == nil || len(key) == 0 { + return nil, models.ErrDatastoreEmptyKey + } + row := ds.db.QueryRow("SELECT value FROM extras WHERE key=$1", key) var value string diff --git a/api/datastore/postgres_test.go b/api/datastore/postgres_test.go index a077cca5d..0eab68bbe 100644 --- a/api/datastore/postgres_test.go +++ b/api/datastore/postgres_test.go @@ -49,18 +49,6 @@ func TestPostgres(t *testing.T) { t.Fatalf("Error when creating datastore: %v", err) } - testApp := &models.App{ - Name: "Test", - } - - testRoute := &models.Route{ - AppName: testApp.Name, - Path: "/test", - Image: "iron/hello", - Type: "sync", - Format: "http", - } - // Testing insert app _, err = ds.InsertApp(ctx, nil) if err != models.ErrDatastoreEmptyApp { @@ -288,4 +276,48 @@ func TestPostgres(t *testing.T) { t.Log(buf.String()) t.Fatalf("Test RemoveApp: failed to remove the route") } + + // Testing Put/Get + err = ds.Put(ctx, nil, nil) + if err != models.ErrDatastoreEmptyKey { + t.Log(buf.String()) + t.Fatalf("Test Put(nil,nil): expected error `%v`, but it was `%v`", models.ErrDatastoreEmptyKey, err) + } + + err = ds.Put(ctx, []byte("test"), []byte("success")) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + + val, err := ds.Get(ctx, []byte("test")) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + if string(val) != "success" { + t.Log(buf.String()) + t.Fatalf("Test Get: expected value to be `%v`, but it was `%v`", "success", string(val)) + } + + err = ds.Put(ctx, []byte("test"), nil) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + + val, err = ds.Get(ctx, []byte("test")) + if err != nil { + t.Log(buf.String()) + t.Fatalf("Test Put: unexpected error: %v", err) + } + if string(val) != "" { + t.Log(buf.String()) + t.Fatalf("Test Get: expected value to be `%v`, but it was `%v`", "", string(val)) + } + +} + +func testPostgresInsert(t *testing.T, ctx context.Context, ds models.Datastore) { + } diff --git a/api/models/datastore.go b/api/models/datastore.go index 9aee718c1..1e3f3a82f 100644 --- a/api/models/datastore.go +++ b/api/models/datastore.go @@ -30,4 +30,5 @@ var ( ErrDatastoreEmptyRoutePath = errors.New("Missing route name") ErrDatastoreEmptyApp = errors.New("Missing app") ErrDatastoreEmptyRoute = errors.New("Missing route") + ErrDatastoreEmptyKey = errors.New("Missing key") )