Repository: ovn-org/libovsdb (go)

Issue:
Remove Client.API
It is much more fluent to be able to call `client.Get()` instead of `client.API.Get()`

Pull request:
Implement the API interface in ovsdbClient
Makes the library more user friendly

Fixes #102 

Patch:
diff --git a/client/client.go b/client/client.go
index ffb5ef0..67bd9f0 100644
--- a/client/client.go
+++ b/client/client.go
@@ -23,7 +23,7 @@ type OvsdbClient struct {
 	handlersMutex *sync.Mutex
 	Cache         *TableCache
 	stopCh        chan struct{}
-	API           API
+	api           API
 }
 
 func newOvsdbClient() *OvsdbClient {
@@ -131,7 +131,7 @@ func newRPC2Client(conn net.Conn, database *DBModel) (*OvsdbClient, error) {
 		if cache, err := newTableCache(schema, database); err == nil {
 			ovs.Cache = cache
 			ovs.Register(ovs.Cache)
-			ovs.API = newAPI(ovs.Cache)
+			ovs.api = newAPI(ovs.Cache)
 		} else {
 			ovs.rpcClient.Close()
 			return nil, err
@@ -347,3 +347,40 @@ func (ovs OvsdbClient) Disconnect() {
 	close(ovs.stopCh)
 	ovs.rpcClient.Close()
 }
+
+// Client API interface wrapper functions
+// We add this wrapper to allow users to access the API directly on the
+// client object
+
+// Ensure client implementes API
+var _ API = OvsdbClient{}
+
+//Get implements the API interface's Get function
+func (ovs OvsdbClient) Get(model Model) error {
+	return ovs.api.Get(model)
+}
+
+//Create implementes the API interface's Create function
+func (ovs OvsdbClient) Create(model Model) (*ovsdb.Operation, error) {
+	return ovs.api.Create(model)
+}
+
+//List implements the API interface's List function
+func (ovs OvsdbClient) List(result interface{}) error {
+	return ovs.api.List(result)
+}
+
+//Where implements the API interface's Where function
+func (ovs OvsdbClient) Where(condition Condition) ConditionalAPI {
+	return ovs.api.Where(condition)
+}
+
+//ConditionFromFunc implements the API interface's ConditionFromFunc function
+func (ovs OvsdbClient) ConditionFromFunc(predicate interface{}) Condition {
+	return ovs.api.ConditionFromFunc(predicate)
+}
+
+//ConditionFromModel implements the API interface's ConditionFromModel function
+func (ovs OvsdbClient) ConditionFromModel(m Model, fields ...interface{}) Condition {
+	return ovs.api.ConditionFromModel(m, fields...)
+}
diff --git a/cmd/modelgen/dbmodel.go b/cmd/modelgen/dbmodel.go
index 2bd8603..28948f8 100644
--- a/cmd/modelgen/dbmodel.go
+++ b/cmd/modelgen/dbmodel.go
@@ -13,12 +13,12 @@ const MODEL_TEMPLATE = `
 package {{ .PackageName }}
 
 import (
-	"github.com/ovn-org/libovsdb/client"
+	goovn "github.com/ovn-org/libovsdb"
 )
 
 // FullDatabaseModel() returns the DatabaseModel object to be used in libovsdb
-func FullDatabaseModel() (*client.DBModel, error) {
-	return client.NewDBModel("{{ .DatabaseName }}", map[string]client.Model{
+func FullDatabaseModel() (*goovn.DBModel, error) {
+	return goovn.NewDBModel("{{ .DatabaseName }}", map[string]goovn.Model{
     {{ range $tableName, $structName := .Tables }} "{{ $tableName }}" : &{{ $structName }}{}, 
     {{ end }}
 	})
diff --git a/cmd/modelgen/main.go b/cmd/modelgen/main.go
index 4f99049..62c6155 100644
--- a/cmd/modelgen/main.go
+++ b/cmd/modelgen/main.go
@@ -50,7 +50,7 @@ func main() {
 		log.Fatal(err)
 	}
 
-	if err := os.MkdirAll(outDir, 0755); err != nil {
+	if err := os.MkdirAll(filepath.Join(outDir, pkgName), 0700); err != nil {
 		log.Fatal(err)
 	}
 
@@ -86,7 +86,7 @@ func main() {
 		if err != nil {
 			log.Fatal(err)
 		}
-		outFile := filepath.Join(outDir, gen.FileName())
+		outFile := filepath.Join(outDir, pkgName, gen.FileName())
 		if err := writeFile(outFile, code); err != nil {
 			log.Fatal(err)
 		}
diff --git a/cmd/modelgen/table.go b/cmd/modelgen/table.go
index 4a92383..7261b3b 100644
--- a/cmd/modelgen/table.go
+++ b/cmd/modelgen/table.go
@@ -2,7 +2,6 @@ package main
 
 import (
 	"fmt"
-	"sort"
 	"strings"
 	"text/template"
 	"unicode"
@@ -54,16 +53,7 @@ func NewTableGenerator(pkg string, name string, table *ovsdb.TableSchema) Genera
 			Tag:  Tag("_uuid"),
 		})
 
-	// Map iteration order is random, so for predictable generation
-	// lets sort fields by name
-	var order sort.StringSlice
-	for columnName := range table.Columns {
-		order = append(order, columnName)
-	}
-	order.Sort()
-
-	for _, columnName := range order {
-		columnSchema := table.Columns[columnName]
+	for columnName, columnSchema := range table.Columns {
 		templateData.Fields = append(templateData.Fields, Field{
 			Name: FieldName(columnName),
 			Type: FieldType(columnSchema),
diff --git a/cmd/stress/stress.go b/cmd/stress/stress.go
index 2ea2cd6..5176524 100644
--- a/cmd/stress/stress.go
+++ b/cmd/stress/stress.go
@@ -72,7 +72,7 @@ func run() {
 
 	// Remove all existing bridges
 	var bridges []ormBridge
-	if err := ovs.API.List(&bridges); err != nil {
+	if err := ovs.List(&bridges); err != nil {
 		log.Fatal(err)
 	}
 	for _, bridge := range bridges {
@@ -105,7 +105,7 @@ func transact(ovs *client.OvsdbClient, operations []ovsdb.Operation) (ok bool, u
 }
 
 func deleteBridge(ovs *client.OvsdbClient, bridge *ormBridge) {
-	deleteOp, err := ovs.API.Where(ovs.API.ConditionFromModel(bridge)).Delete()
+	deleteOp, err := ovs.Where(ovs.ConditionFromModel(bridge)).Delete()
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -113,7 +113,7 @@ func deleteBridge(ovs *client.OvsdbClient, bridge *ormBridge) {
 		UUID: rootUUID,
 	}
 
-	mutateOp, err := ovs.API.Where(ovs.API.ConditionFromModel(&ovsRow)).Mutate(&ovsRow, []client.Mutation{
+	mutateOp, err := ovs.Where(ovs.ConditionFromModel(&ovsRow)).Mutate(&ovsRow, []client.Mutation{
 		{
 			Field:   &ovsRow.Bridges,
 			Mutator: "delete",
@@ -146,12 +146,12 @@ func createBridge(ovs *client.OvsdbClient, iter int) {
 			"key2": "val2",
 		},
 	}
-	insertOp, err := ovs.API.Create(&bridge)
+	insertOp, err := ovs.Create(&bridge)
 	if err != nil {
 		log.Fatal(err)
 	}
 	ovsRow := ormOvs{}
-	mutateOp, err := ovs.API.Where(ovs.API.ConditionFromModel(&ormOvs{UUID: rootUUID})).Mutate(&ovsRow, []client.Mutation{
+	mutateOp, err := ovs.Where(ovs.ConditionFromModel(&ormOvs{UUID: rootUUID})).Mutate(&ovsRow, []client.Mutation{
 		{
 			Field:   &ovsRow.Bridges,
 			Mutator: "insert",
diff --git a/example/play_with_ovs/play_with_ovs.go b/example/play_with_ovs/play_with_ovs.go
index 3502f4f..0410016 100644
--- a/example/play_with_ovs/play_with_ovs.go
+++ b/example/play_with_ovs/play_with_ovs.go
@@ -49,7 +49,7 @@ func play(ovs *client.OvsdbClient) {
 		} else {
 			fmt.Printf("Current list of bridges:\n")
 			var bridges []ormBridge
-			if err := ovs.API.List(&bridges); err != nil {
+			if err := ovs.List(&bridges); err != nil {
 				log.Fatal(err)
 			}
 			for _, b := range bridges {
@@ -64,7 +64,7 @@ func createBridge(ovs *client.OvsdbClient, bridgeName string) {
 		UUID: "gopher",
 		Name: bridgeName,
 	}
-	insertOp, err := ovs.API.Create(&bridge)
+	insertOp, err := ovs.Create(&bridge)
 	if err != nil {
 		log.Fatal(err)
 	}
@@ -72,7 +72,7 @@ func createBridge(ovs *client.OvsdbClient, bridgeName string) {
 	ovsRow := ormOvs{
 		UUID: rootUUID,
 	}
-	mutateOps, err := ovs.API.Where(ovs.API.ConditionFromModel(&ovsRow)).Mutate(&ovsRow, []client.Mutation{
+	mutateOps, err := ovs.Where(ovs.ConditionFromModel(&ovsRow)).Mutate(&ovsRow, []client.Mutation{
 		{
 			Field:   &ovsRow.Bridges,
 			Mutator: "insert",


Test patch:
diff --git a/client/ovs_integration_test.go b/client/ovs_integration_test.go
index 3e6d8d5..692070b 100644
--- a/client/ovs_integration_test.go
+++ b/client/ovs_integration_test.go
@@ -9,6 +9,7 @@ import (
 	"time"
 
 	"github.com/ovn-org/libovsdb/ovsdb"
+	"github.com/stretchr/testify/assert"
 )
 
 const (
@@ -16,12 +17,29 @@ const (
 	defOvsSocket = "db.sock"
 )
 
-var defDB = &DBModel{name: "Open_vSwitch"}
+// ORMBridge is the simplified ORM model of the Bridge table
+type bridgeType struct {
+	UUID        string            `ovs:"_uuid"`
+	Name        string            `ovs:"name"`
+	OtherConfig map[string]string `ovs:"other_config"`
+	ExternalIds map[string]string `ovs:"external_ids"`
+	Ports       []string          `ovs:"ports"`
+	Status      map[string]string `ovs:"status"`
+}
+
+// ORMovs is the simplified ORM model of the Bridge table
+type ovsType struct {
+	UUID    string   `ovs:"_uuid"`
+	Bridges []string `ovs:"bridges"`
+}
+
+var defDB, _ = NewDBModel("Open_vSwitch", map[string]Model{
+	"Open_vSwitch": &ovsType{},
+	"Bridge":       &bridgeType{}})
 
 var cfg *Config
 
 func SetConfig() {
-
 	cfg = &Config{}
 	var ovsRunDir = os.Getenv("OVS_RUNDIR")
 	if ovsRunDir == "" {
@@ -150,54 +168,41 @@ func TestInsertTransactIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
 		t.Fatalf("Failed to Connect. error: %s", err)
 	}
+	err = ovs.MonitorAll(nil)
+	assert.Nil(t, err)
 
 	// NamedUUID is used to add multiple related Operations in a single Transact operation
 	namedUUID := "gopher"
-
-	externalIds := make(map[string]string)
-	externalIds["go"] = "awesome"
-	externalIds["docker"] = "made-for-each-other"
-	oMap, err := ovsdb.NewOvsMap(externalIds)
-	if err != nil {
-		t.Fatal(err)
+	br := bridgeType{
+		UUID: namedUUID,
+		Name: bridgeName,
+		ExternalIds: map[string]string{
+			"go":     "awesome",
+			"docker": "made-for-each-other",
+		},
 	}
-	// bridge row to insert
-	bridge := make(map[string]interface{})
-	bridge["name"] = bridgeName
-	bridge["external_ids"] = oMap
 
-	// simple insert operation
-	insertOp := ovsdb.Operation{
-		Op:       "insert",
-		Table:    "Bridge",
-		Row:      bridge,
-		UUIDName: namedUUID,
-	}
+	insertOp, err := ovs.Create(&br)
+	assert.Nil(t, err)
 
 	// Inserting a Bridge row in Bridge table requires mutating the open_vswitch table.
-	mutateUUID := []ovsdb.UUID{{GoUUID: namedUUID}}
-	mutateSet, _ := ovsdb.NewOvsSet(mutateUUID)
-	mutation := ovsdb.NewMutation("bridges", "insert", mutateSet)
-	// hacked Condition till we get Monitor / Select working
-	condition := ovsdb.NewCondition("_uuid", "!=", ovsdb.UUID{GoUUID: "2f77b348-9768-4866-b761-89d5177ecdab"})
-
-	// simple mutate operation
-	mutateOp := ovsdb.Operation{
-		Op:        "mutate",
-		Table:     "Open_vSwitch",
-		Mutations: []interface{}{mutation},
-		Where:     []interface{}{condition},
-	}
-
-	operations := []ovsdb.Operation{insertOp, mutateOp}
+	ovsRow := ovsType{}
+	mutateOp, err := ovs.Where(ovs.ConditionFromFunc(func(*ovsType) bool { return true })).
+		Mutate(&ovsRow, []Mutation{
+			{
+				Field:   &ovsRow.Bridges,
+				Mutator: ovsdb.MutateOperationInsert,
+				Value:   []string{namedUUID},
+			},
+		})
+	assert.Nil(t, err)
+
+	operations := append([]ovsdb.Operation{*insertOp}, mutateOp...)
 	reply, err := ovs.Transact(operations...)
 	if err != nil {
 		t.Fatal(err)
@@ -229,10 +234,6 @@ func TestDeleteTransactIntegration(t *testing.T) {
 	}
 	SetConfig()
 
-	if testing.Short() {
-		t.Skip()
-	}
-
 	if bridgeUUID == "" {
 		t.Skip()
 	}
@@ -241,31 +242,25 @@ func TestDeleteTransactIntegration(t *testing.T) {
 	if err != nil {
 		t.Fatalf("Failed to Connect. error: %s", err)
 	}
+	err = ovs.MonitorAll(nil)
+	assert.Nil(t, err)
 
-	// simple delete operation
-	condition := ovsdb.NewCondition("name", "==", bridgeName)
-	deleteOp := ovsdb.Operation{
-		Op:    "delete",
-		Table: "Bridge",
-		Where: []interface{}{condition},
-	}
+	deleteOp, err := ovs.Where(ovs.ConditionFromModel(&bridgeType{Name: bridgeName})).Delete()
+	assert.Nil(t, err)
 
-	// Deleting a Bridge row in Bridge table requires mutating the open_vswitch table.
-	mutateUUID := []ovsdb.UUID{{GoUUID: bridgeUUID}}
-	mutateSet, _ := ovsdb.NewOvsSet(mutateUUID)
-	mutation := ovsdb.NewMutation("bridges", "delete", mutateSet)
-	// hacked Condition till we get Monitor / Select working
-	condition = ovsdb.NewCondition("_uuid", "!=", ovsdb.UUID{GoUUID: "2f77b348-9768-4866-b761-89d5177ecdab"})
+	ovsRow := ovsType{}
+	mutateOp, err := ovs.Where(ovs.ConditionFromFunc(func(*ovsType) bool { return true })).
+		Mutate(&ovsRow, []Mutation{
+			{
+				Field:   &ovsRow.Bridges,
+				Mutator: ovsdb.MutateOperationDelete,
+				Value:   []string{bridgeUUID},
+			},
+		})
 
-	// simple mutate operation
-	mutateOp := ovsdb.Operation{
-		Op:        "mutate",
-		Table:     "Open_vSwitch",
-		Mutations: []interface{}{mutation},
-		Where:     []interface{}{condition},
-	}
+	assert.Nil(t, err)
 
-	operations := []ovsdb.Operation{deleteOp, mutateOp}
+	operations := append(deleteOp, mutateOp...)
 	reply, err := ovs.Transact(operations...)
 	if err != nil {
 		t.Fatal(err)
@@ -316,9 +311,6 @@ func TestNotifyIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
@@ -350,9 +342,6 @@ func TestRemoveNotifyIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
@@ -396,9 +385,6 @@ func TestTableSchemaValidationIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
@@ -427,9 +413,6 @@ func TestColumnSchemaInRowValidationIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
@@ -460,9 +443,6 @@ func TestColumnSchemaInMultipleRowsValidationIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
@@ -526,9 +506,6 @@ func TestMonitorCancelIntegration(t *testing.T) {
 		t.Skip("skipping integration test")
 	}
 	SetConfig()
-	if testing.Short() {
-		t.Skip()
-	}
 
 	ovs, err := Connect(cfg.Addr, defDB, nil)
 	if err != nil {
diff --git a/cmd/modelgen/table_test.go b/cmd/modelgen/table_test.go
deleted file mode 100644
index 1483f31..0000000
--- a/cmd/modelgen/table_test.go
+++ /dev/null
@@ -1,132 +0,0 @@
-package main
-
-import (
-	"encoding/json"
-	"testing"
-
-	"github.com/ovn-org/libovsdb/ovsdb"
-	"github.com/stretchr/testify/assert"
-)
-
-func TestNewTableGenerator(t *testing.T) {
-	rawSchema := []byte(`
-	{
-		"name": "AtomicDB",
-		"version": "0.0.0",
-		"tables": {
-			"atomicTable": {
-				"columns": {
-					"str": {
-						"type": "string"
-					},
-					"int": {
-						"type": "integer"
-					},
-					"float": {
-						"type": "real"
-					}
-				}
-			}
-		}
-	}`)
-
-	expected := `// Code generated by "ovsdb.modelgen"
-// DO NOT EDIT.
-
-package test
-
-// test defines an object in test table
-type test struct {
-	UUID  string  ` + "`" + `ovs:"_uuid"` + "`" + `
-	Float float64 ` + "`" + `ovs:"float"` + "`" + `
-	Int   int     ` + "`" + `ovs:"int"` + "`" + `
-	Str   string  ` + "`" + `ovs:"str"` + "`" + `
-}
-`
-
-	var schema ovsdb.DatabaseSchema
-	err := json.Unmarshal(rawSchema, &schema)
-	if err != nil {
-		t.Fatal(err)
-	}
-	table := schema.Tables["atomicTable"]
-	gen := NewTableGenerator(
-		"test",
-		"test",
-		&table,
-	)
-
-	for i := 0; i < 3; i++ {
-		b, err := gen.Format()
-		if err != nil {
-			t.Fatal(err)
-		}
-		assert.Equal(t, expected, string(b))
-	}
-}
-
-func TestFieldName(t *testing.T) {
-	if s := FieldName("foo"); s != "Foo" {
-		t.Fatalf("got %s, wanted Foo", s)
-	}
-}
-
-func TestStructName(t *testing.T) {
-	if s := StructName("Foo_Bar"); s != "FooBar" {
-		t.Fatalf("got %s, wanted FooBar", s)
-	}
-}
-
-/*
-TODO: Write Test
-func TestFieldType(t *testing.T) {
-	tests := []struct {
-		name string
-		in   *ovsdb.ColumnSchema
-		out  string
-	}
-	if got := FieldType(tt.args.column); got != tt.want {
-		t.Errorf("FieldType() = %v, want %v", got, tt.want)
-	}
-}
-*/
-
-func TestAtomicType(t *testing.T) {
-	tests := []struct {
-		name string
-		in   string
-		out  string
-	}{
-		{"IntegerToInt", ovsdb.TypeInteger, "int"},
-		{"RealToFloat", ovsdb.TypeReal, "float64"},
-		{"BooleanToBool", ovsdb.TypeBoolean, "bool"},
-		{"StringToString", ovsdb.TypeString, "string"},
-		{"UUIDToString", ovsdb.TypeUUID, "string"},
-		{"Invalid", "notAType", ""},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := AtomicType(tt.in); got != tt.out {
-				t.Errorf("got %s, wanted %s", got, tt.out)
-			}
-		})
-	}
-}
-
-func TestTag(t *testing.T) {
-	if s := Tag("Foo_Bar"); s != "`ovs:\"Foo_Bar\"`" {
-		t.Fatalf("got %s, wanted `ovs:\"Foo_Bar\"`", s)
-	}
-}
-
-func TestFileName(t *testing.T) {
-	if s := FileName("foo"); s != "foo.go" {
-		t.Fatalf("got %s, wanted foo.go", s)
-	}
-}
-
-func TestCamelCase(t *testing.T) {
-	if s := camelCase("foo_bar_baz"); s != "FooBarBaz" {
-		t.Fatalf("got %s, wanted FooBarBaz", s)
-	}
-}