Golint Fixes and Build Improvements

- Update Makefile - Tests now use Docker Compose - No more rewritten import paths - Golint Fixes Signed-off-by: Dave Tucker <[email protected]>
hzhou8 · May 3, 2016 · 139587a · 139587a
1 parent f6ba96e
commit 139587a
Show file tree

Hide file tree

Showing 13 changed files with 144 additions and 97 deletions.
diff --git a/Makefile b/Makefile
@@ -1,11 +1,28 @@
-all: build test
+.PHONY: all test test-local install-deps lint fmt vet
 
-build:
-	go build -v
+all: test
+
+test-local: install-deps fmt lint vet
+	@echo "+ $@"
+	@go test -v ./...
 
 test:
-	go test -covermode=count -coverprofile=coverage.out -test.short -v
+	@docker-compose run --rm test
+
+install-deps:
+	@echo "+ $@"
+	@go get -u github.com/golang/lint/golint
+	@go get -d ./...
+
+lint:
+	@echo "+ $@"
+	@test -z "$$(golint ./... | tee /dev/stderr)"
+
+fmt:
+	@echo "+ $@"
+	@test -z "$$(gofmt -s -l . | tee /dev/stderr)"
 
-test-all:
-	go test -covermode=count -coverprofile=coverage.out -v
+vet:
+	@echo "+ $@"
+	@go vet ./...
 
diff --git a/client.go b/client.go
@@ -8,10 +8,11 @@ import (
 	"net"
 	"sync"
 
-	"github.com/socketplane/libovsdb/Godeps/_workspace/src/github.com/cenkalti/rpc2"
-	"github.com/socketplane/libovsdb/Godeps/_workspace/src/github.com/cenkalti/rpc2/jsonrpc"
+	"github.com/cenk/rpc2"
+	"github.com/cenk/rpc2/jsonrpc"
 )
 
+// OvsdbClient is an OVSDB client
 type OvsdbClient struct {
 	rpcClient *rpc2.Client
 	Schema    map[string]DatabaseSchema
@@ -34,16 +35,20 @@ func newOvsdbClient(c *rpc2.Client) *OvsdbClient {
 var connections map[*rpc2.Client]*OvsdbClient
 var connectionsMutex = &sync.RWMutex{}
 
-const DEFAULT_ADDR = "127.0.0.1"
-const DEFAULT_PORT = 6640
+// DefaultAddress is the default IPV4 address that is used for a connection
+const DefaultAddress = "127.0.0.1"
 
+// DefaultPort is the default port used for a connection
+const DefaultPort = 6640
+
+// Connect creates an OVSDB connection and returns and OvsdbClient
 func Connect(ipAddr string, port int) (*OvsdbClient, error) {
 	if ipAddr == "" {
-		ipAddr = DEFAULT_ADDR
+		ipAddr = DefaultAddress
 	}
 
 	if port <= 0 {
-		port = DEFAULT_PORT
+		port = DefaultPort
 	}
 
 	target := fmt.Sprintf("%s:%d", ipAddr, port)
@@ -76,10 +81,12 @@ func Connect(ipAddr string, port int) (*OvsdbClient, error) {
 	return ovs, nil
 }
 
+// Register registers the supplied NotificationHandler to recieve OVSDB Notifications
 func (ovs *OvsdbClient) Register(handler NotificationHandler) {
 	ovs.handlers = append(ovs.handlers, handler)
 }
 
+// NotificationHandler is the interface that must be implemented to receive notifcations
 type NotificationHandler interface {
 	// RFC 7047 section 4.1.6 Update Notification
 	Update(context interface{}, tableUpdates TableUpdates)
@@ -145,19 +152,20 @@ func update(client *rpc2.Client, params []interface{}, reply *interface{}) error
 	return nil
 }
 
+// GetSchema returns the schema in use for the provided database name
 // RFC 7047 : get_schema
 func (ovs OvsdbClient) GetSchema(dbName string) (*DatabaseSchema, error) {
 	args := NewGetSchemaArgs(dbName)
 	var reply DatabaseSchema
 	err := ovs.rpcClient.Call("get_schema", args, &reply)
 	if err != nil {
 		return nil, err
-	} else {
-		ovs.Schema[dbName] = reply
 	}
+	ovs.Schema[dbName] = reply
 	return &reply, err
 }
 
+// ListDbs returns the list of databases on the server
 // RFC 7047 : list_dbs
 func (ovs OvsdbClient) ListDbs() ([]string, error) {
 	var dbs []string
@@ -168,8 +176,8 @@ func (ovs OvsdbClient) ListDbs() ([]string, error) {
 	return dbs, err
 }
 
+// Transact performs the provided Operation's on the database
 // RFC 7047 : transact
-
 func (ovs OvsdbClient) Transact(database string, operation ...Operation) ([]OperationResult, error) {
 	var reply []OperationResult
 	db, ok := ovs.Schema[database]
@@ -189,7 +197,7 @@ func (ovs OvsdbClient) Transact(database string, operation ...Operation) ([]Oper
 	return reply, nil
 }
 
-// Convenience method to monitor every table/column
+// MonitorAll is a convenience method to monitor every table/column
 func (ovs OvsdbClient) MonitorAll(database string, jsonContext interface{}) (*TableUpdates, error) {
 	schema, ok := ovs.Schema[database]
 	if !ok {
@@ -199,7 +207,7 @@ func (ovs OvsdbClient) MonitorAll(database string, jsonContext interface{}) (*Ta
 	requests := make(map[string]MonitorRequest)
 	for table, tableSchema := range schema.Tables {
 		var columns []string
-		for column, _ := range tableSchema.Columns {
+		for column := range tableSchema.Columns {
 			columns = append(columns, column)
 		}
 		requests[table] = MonitorRequest{
@@ -214,6 +222,7 @@ func (ovs OvsdbClient) MonitorAll(database string, jsonContext interface{}) (*Ta
 	return ovs.Monitor(database, jsonContext, requests)
 }
 
+// Monitor will provide updates for a given table/column
 // RFC 7047 : monitor
 func (ovs OvsdbClient) Monitor(database string, jsonContext interface{}, requests map[string]MonitorRequest) (*TableUpdates, error) {
 	var reply TableUpdates
@@ -261,6 +270,7 @@ func handleDisconnectNotification(c *rpc2.Client) {
 	}
 }
 
+// Disconnect will close the OVSDB connection
 func (ovs OvsdbClient) Disconnect() {
 	ovs.rpcClient.Close()
 	clearConnection(ovs.rpcClient)

diff --git a/docker-compose.yml b/docker-compose.yml
@@ -0,0 +1,17 @@
+ovs:
+  image: socketplane/openvswitch:2.4.0
+  ports:
+    - "6640:6640"
+  command: "/usr/bin/supervisord -n"
+  privileged: true
+
+test:
+  image: golang:1.6
+  links:
+    - ovs
+  volumes: 
+    - .:/go/src/github.com/socketplane/libovsdb
+  working_dir: /go/src/github.com/socketplane/libovsdb
+  environment:
+    DOCKER_IP: "ovs"
+  command: "make test-local"
diff --git a/example/play_with_ovs.go b/example/play_with_ovs.go
@@ -41,7 +41,7 @@ func play(ovs *libovsdb.OvsdbClient) {
 }
 
 func createBridge(ovs *libovsdb.OvsdbClient, bridgeName string) {
-	namedUuid := "gopher"
+	namedUUID := "gopher"
 	// bridge row to insert
 	bridge := make(map[string]interface{})
 	bridge["name"] = bridgeName
@@ -51,14 +51,14 @@ func createBridge(ovs *libovsdb.OvsdbClient, bridgeName string) {
 		Op:       "insert",
 		Table:    "Bridge",
 		Row:      bridge,
-		UUIDName: namedUuid,
+		UUIDName: namedUUID,
 	}
 
 	// Inserting a Bridge row in Bridge table requires mutating the open_vswitch table.
-	mutateUuid := []libovsdb.UUID{libovsdb.UUID{namedUuid}}
-	mutateSet, _ := libovsdb.NewOvsSet(mutateUuid)
+	mutateUUID := []libovsdb.UUID{{namedUUID}}
+	mutateSet, _ := libovsdb.NewOvsSet(mutateUUID)
 	mutation := libovsdb.NewMutation("bridges", "insert", mutateSet)
-	condition := libovsdb.NewCondition("_uuid", "==", libovsdb.UUID{getRootUuid()})
+	condition := libovsdb.NewCondition("_uuid", "==", libovsdb.UUID{getRootUUID()})
 
 	// simple mutate operation
 	mutateOp := libovsdb.Operation{
@@ -85,7 +85,7 @@ func createBridge(ovs *libovsdb.OvsdbClient, bridgeName string) {
 		}
 	}
 	if ok {
-		fmt.Println("Bridge Addition Successful : ", reply[0].UUID.GoUuid)
+		fmt.Println("Bridge Addition Successful : ", reply[0].UUID.GoUUID)
 	}
 }
 
@@ -98,8 +98,8 @@ func processInput(ovs *libovsdb.OvsdbClient) {
 	}
 }
 
-func getRootUuid() string {
-	for uuid, _ := range cache["Open_vSwitch"] {
+func getRootUUID() string {
+	for uuid := range cache["Open_vSwitch"] {
 		return uuid
 	}
 	return ""
@@ -137,7 +137,7 @@ func main() {
 		fmt.Println("Unable to Connect ", err)
 		os.Exit(1)
 	}
-	var notifier Notifier
+	var notifier myNotifier
 	ovs.Register(notifier)
 
 	initial, _ := ovs.MonitorAll("Open_vSwitch", "")
@@ -148,18 +148,18 @@ func main() {
 	<-quit
 }
 
-type Notifier struct {
+type myNotifier struct {
 }
 
-func (n Notifier) Update(context interface{}, tableUpdates libovsdb.TableUpdates) {
+func (n myNotifier) Update(context interface{}, tableUpdates libovsdb.TableUpdates) {
 	populateCache(tableUpdates)
 	update <- &tableUpdates
 }
-func (n Notifier) Locked([]interface{}) {
+func (n myNotifier) Locked([]interface{}) {
 }
-func (n Notifier) Stolen([]interface{}) {
+func (n myNotifier) Stolen([]interface{}) {
 }
-func (n Notifier) Echo([]interface{}) {
+func (n myNotifier) Echo([]interface{}) {
 }
-func (n Notifier) Disconnected(client *libovsdb.OvsdbClient) {
+func (n myNotifier) Disconnected(client *libovsdb.OvsdbClient) {
 }
diff --git a/fig.yml b/fig.yml
diff --git a/map.go b/map.go
@@ -6,18 +6,18 @@ import (
 	"reflect"
 )
 
-//  RFC 7047 uses the following notation for map as JSON doesnt support non-string keys for maps.
-//  A 2-element JSON array that represents a database map value.  The
-//  first element of the array must be the string "map", and the
-//  second element must be an array of zero or more <pair>s giving the
-//  values in the map.  All of the <pair>s must have the same key and
-//  value types.
-
+// OvsMap is the JSON map structure used for OVSDB
+// RFC 7047 uses the following notation for map as JSON doesnt support non-string keys for maps.
+// A 2-element JSON array that represents a database map value.  The
+// first element of the array must be the string "map", and the
+// second element must be an array of zero or more <pair>s giving the
+// values in the map.  All of the <pair>s must have the same key and
+// value types.
 type OvsMap struct {
 	GoMap map[interface{}]interface{}
 }
 
-// <map> notation requires special handling
+// MarshalJSON marshalls an OVSDB style Map to a byte array
 func (o OvsMap) MarshalJSON() ([]byte, error) {
 	var ovsMap, innerMap []interface{}
 	ovsMap = append(ovsMap, "map")
@@ -31,6 +31,7 @@ func (o OvsMap) MarshalJSON() ([]byte, error) {
 	return json.Marshal(ovsMap)
 }
 
+// UnmarshalJSON unmarshalls an OVSDB style Map from a byte array
 func (o *OvsMap) UnmarshalJSON(b []byte) (err error) {
 	var oMap []interface{}
 	o.GoMap = make(map[interface{}]interface{})
@@ -44,7 +45,7 @@ func (o *OvsMap) UnmarshalJSON(b []byte) (err error) {
 	return err
 }
 
-// <map> notation requires special marshaling
+// NewOvsMap will return an OVSDB style map from a provided Golang Map
 func NewOvsMap(goMap interface{}) (*OvsMap, error) {
 	v := reflect.ValueOf(goMap)
 	if v.Kind() != reflect.Map {

diff --git a/notation.go b/notation.go
@@ -16,13 +16,10 @@ type Operation struct {
 	UUIDName  string                   `json:"uuid-name,omitempty"`
 }
 
-// MonitorRequest represents a monitor request according to RFC7047
-/*
- * We cannot use MonitorRequests by inlining the MonitorRequest Map structure till GoLang issue #6213 makes it.
- * The only option is to go with raw map[string]interface{} option :-( that sucks !
- * Refer to client.go : MonitorAll() function for more details
- */
-
+// MonitorRequests represents a group of monitor requests according to RFC7047
+// We cannot use MonitorRequests by inlining the MonitorRequest Map structure till GoLang issue #6213 makes it.
+// The only option is to go with raw map[string]interface{} option :-( that sucks !
+// Refer to client.go : MonitorAll() function for more details
 type MonitorRequests struct {
 	Requests map[string]MonitorRequest `json:"requests,overflow"`
 }
@@ -41,23 +38,23 @@ type MonitorSelect struct {
 	Modify  bool `json:"modify,omitempty"`
 }
 
-/*
- * We cannot use TableUpdates directly by json encoding by inlining the TableUpdate Map
- * structure till GoLang issue #6213 makes it.
- *
- * The only option is to go with raw map[string]map[string]interface{} option :-( that sucks !
- * Refer to client.go : MonitorAll() function for more details
- */
+// TableUpdates is a collection of TableUpdate entries
+// We cannot use TableUpdates directly by json encoding by inlining the TableUpdate Map
+// structure till GoLang issue #6213 makes it.
+// The only option is to go with raw map[string]map[string]interface{} option :-( that sucks !
+// Refer to client.go : MonitorAll() function for more details
 type TableUpdates struct {
 	Updates map[string]TableUpdate `json:"updates,overflow"`
 }
 
+// TableUpdate represents a table update according to RFC7047
 type TableUpdate struct {
 	Rows map[string]RowUpdate `json:"rows,overflow"`
 }
 
+// RowUpdate represents a row update according to RFC7047
 type RowUpdate struct {
-	Uuid UUID `json:"-,omitempty"`
+	UUID UUID `json:"-,omitempty"`
 	New  Row  `json:"new,omitempty"`
 	Old  Row  `json:"old,omitempty"`
 }
@@ -78,11 +75,13 @@ func NewMutation(column string, mutator string, value interface{}) []interface{}
 	return []interface{}{column, mutator, value}
 }
 
+// TransactResponse represents the response to a Transact Operation
 type TransactResponse struct {
 	Result []OperationResult `json:"result"`
 	Error  string            `json:"error"`
 }
 
+// OperationResult is the result of an Operation
 type OperationResult struct {
 	Count   int                      `json:"count,omitempty"`
 	Error   string                   `json:"error,omitempty"`