Repository: ovn-org/libovsdb (go)

Issue:
List() does not work on empty Lists
Currently List() respects the capacity of the given list and does not resize it. This is intended so that a client that preallocates a list for optimization purposes is not returned an increased slice.
However, the side efect is that a zero capacity (`[]MyRouter{}`) slice does not get populated.
We should consider a slice with capacity==0 in the same way as an nil slice

Pull request description:
api: list all rows if given a empty non-nil slice
This case clearly does not mean the user is trying to do memory
optimizations, so consider it as if it were a nil slice

Fixes #97

Signed-off-by: Adrian Moreno <amorenoz@redhat.com>

Patch:
diff --git a/client/api.go b/client/api.go
index 55a913d..32a3d7b 100644
--- a/client/api.go
+++ b/client/api.go
@@ -144,7 +144,7 @@ func (a api) List(result interface{}) error {
 
 	// If given a null slice, fill it in the cache table completely, if not, just up to
 	// its capability
-	if resultVal.IsNil() {
+	if resultVal.IsNil() || resultVal.Cap() == 0 {
 		resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, tableCache.Len()))
 	}
 	i := resultVal.Len()
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),


Test patch:
diff --git a/client/api_test.go b/client/api_test.go
index 827451e..c168fd4 100644
--- a/client/api_test.go
+++ b/client/api_test.go
@@ -108,6 +108,14 @@ func TestAPIListSimple(t *testing.T) {
 		assert.Nil(t, err)
 		assert.Len(t, result, 0, "Should be empty since cache is empty")
 	})
+
+	t.Run("ApiList: Empty List", func(t *testing.T) {
+		result := []testLogicalSwitch{}
+		api := newAPI(cache)
+		err := api.List(&result)
+		assert.Nil(t, err)
+		assert.Len(t, result, len(lscacheList))
+	})
 }
 
 func TestAPIListPredicate(t *testing.T) {
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)
-	}
-}