added ConfigPassword struct and multithreaded registration test

.travis.yml

@@ -18,7 +18,7 @@ install:
   - cd ..
   - docker cp db-test-seed.sql cacophony-api:/db-seed.sql
 before_script:
-  - ./wait-for-api || { docker ps; docker logs cacophony-api; exit 1; }
+  - ./cacophony-api/wait-for-api || { docker ps; docker logs cacophony-api; exit 1; }
   - sudo docker exec cacophony-api sh -c "sudo -i -u postgres psql cacophonytest -f/db-seed.sql"
 script:
   - go vet ./... && go test ./...

api.go

@@ -93,18 +93,33 @@ func NewAPIFromConfig(configFile string) (*CacophonyAPI, error) {
 		return nil, fmt.Errorf("configuration error: %v", err)
 	}
 	privConfigFilename := privConfigFilename(configFile)
-	password, err := ReadPassword(privConfigFilename)
+	confPassword := NewConfigPassword(privConfigFilename)
+
+	password, err := confPassword.ReadPassword()
 	if err != nil {
 		return nil, err
 	}
 
+	if password == "" {
+		locked, err := confPassword.GetExLock()
+		if locked == false || err != nil {
+			return nil, err
+		}
+		defer confPassword.Unlock()
+
+		//read again incase was just written to while waiting for exlock
+		password, err = confPassword.ReadPassword()
+		if err != nil {
+			return nil, err
+		}
+	}
 	api, err := NewAPI(conf.ServerURL, conf.Group, conf.DeviceName, password)
 	if err != nil {
 		return nil, err
 	}
 
 	if api.JustRegistered() {
-		err := WritePassword(privConfigFilename, api.Password())
+		err := confPassword.WritePassword(api.Password())
 		if err != nil {
 			return nil, err
 		}
@@ -178,9 +193,6 @@ func (api *CacophonyAPI) authenticate() error {
 	if err := d.Decode(&resp); err != nil {
 		return fmt.Errorf("decode: %v", err)
 	}
-	if !resp.Success {
-		return fmt.Errorf("failed getting new token: %v", resp.message())
-	}
 	api.token = resp.Token
 	return nil
 }
@@ -240,9 +252,6 @@ func (api *CacophonyAPI) register() error {
 	if err := d.Decode(&respData); err != nil {
 		return fmt.Errorf("decode: %v", err)
 	}
-	if !respData.Success {
-		return fmt.Errorf("registration failed: %v", respData.message())
-	}
 
 	api.device.password = password
 	api.token = respData.Token
@@ -297,22 +306,17 @@ func (api *CacophonyAPI) UploadThermalRaw(r io.Reader) error {
 }
 
 type tokenResponse struct {
-	Success  bool
 	Messages []string
 	Token    string
 }
 
 // message gets the first message of the supplised tokenResponse if present
-// otherwise default of "unknown" if tokenResponse was unsuccesfull
+// otherwise default of "unknown"
 func (r *tokenResponse) message() string {
 	if len(r.Messages) > 0 {
 		return r.Messages[0]
 	}
-	if r.Success {
-		return ""
-	} else {
-		return "unknown"
-	}
+	return "unknown"
 }
 
 // getFileFromJWT downloads a file from the Cacophony API using supplied JWT

api_test.go

@@ -1,6 +1,4 @@
 // go-api - Client for the Cacophony API server.
-// tests against cacophony-api require apiURL to be pointing
-// to a valid cacophony-api server and test-seed.sql to be run
 // Copyright (C) 2018, The Cacophony Project
 //
 //Licensed under the Apache License, Version 2.0 (the "License");
@@ -18,7 +16,6 @@
 package api
 
 import (
-	"context"
 	"encoding/json"
 	"io"
 	"io/ioutil"
@@ -29,20 +26,25 @@ import (
 	"testing"
 	"time"
 
-	"github.com/gofrs/flock"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	yaml "gopkg.in/yaml.v2"
 )
 
+// tests against cacophony-api require apiURL to be pointing
+// to a valid cacophony-api server and test-seed.sql to be run
+
 var apiURL = "http://localhost:1080"
 var tokenSuccess = true
 var responseHeader = http.StatusOK
 var rawThermalData = randString(100)
+var testConfig = "/var/tmp/go-api-test-config.yaml"
+
 var defaultDevice = "test-device"
 var defaultPassword = "test-password"
 var defaultGroup = "test-group"
 var testEventDetail = `{"description": {"type": "test-id", "details": {"tail":"fuzzy"} } }`
-var tempPasswordFile = "password.tmp"
+var tempPasswordFile = "/var/tmp/password.tmp"
 
 //Tests against httptest
 
@@ -75,7 +77,6 @@ func TestUploadThermalRawHttpRequest(t *testing.T) {
 
 func getTokenResponse() *tokenResponse {
 	return &tokenResponse{
-		Success:  tokenSuccess,
 		Messages: []string{},
 		Token:    "tok-" + randString(20),
 	}
@@ -220,61 +221,100 @@ func TestAPIReportEvent(t *testing.T) {
 	assert.Equal(t, nil, err)
 }
 
-//createTempPasswordFile makes or rewrites supplied filename with password
-func createTempPasswordFile(filename, password string) error {
-	f, err := os.OpenFile(filename, os.O_RDWR|os.O_CREATE, 0775)
-	f.Truncate(0)
-	f.Seek(0, 0)
-
-	defer f.Close()
-	if err != nil {
-		return err
-	}
-	_, err = f.WriteString(password)
-	return err
-}
-
-func readPassword(filename string) (string, error) {
-	content, err := ioutil.ReadFile(filename)
-	return string(content), err
-}
-
-func getLock(filename string) (*flock.Flock, bool, error) {
-	fileLock := flock.New(tempPasswordFile)
-	lockCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
-	defer cancel()
-	locked, err := fileLock.TryLockContext(lockCtx, 678*time.Millisecond)
-	return fileLock, locked, err
-}
-
 func TestPasswordLock(t *testing.T) {
 	tempPassword := randString(20)
-	err := createTempPasswordFile(tempPasswordFile, tempPassword)
-	require.Equal(t, err, nil, "must be able to create/open "+tempPasswordFile)
+	confPassword := NewConfigPassword(tempPasswordFile)
+	anotherConfPassword := NewConfigPassword(tempPasswordFile)
+
+	err := confPassword.WritePassword(tempPassword)
+	assert.NotEqual(t, nil, err)
 
-	fileLock, locked, err := getLock(tempPasswordFile)
+	locked, err := confPassword.GetExLock()
+	defer confPassword.Unlock()
 	require.True(t, locked, "File lock must succeed")
-	require.Equal(t, err, nil, "File lock must succeed")
+	require.Equal(t, nil, err, "must be able to get lock "+tempPasswordFile)
+
+	err = confPassword.WritePassword(tempPassword)
+	require.Equal(t, nil, err, "must be able to write to"+tempPasswordFile)
 
-	err = WritePassword(tempPasswordFile, randString(20))
+	locked, err = anotherConfPassword.GetExLock()
 	assert.NotEqual(t, nil, err)
-	fileLock.Unlock()
+	assert.False(t, locked)
 
-	currentPassword, err := readPassword(tempPasswordFile)
-	assert.Equal(t, err, nil)
+	err = anotherConfPassword.WritePassword(randString(20))
+	assert.NotEqual(t, nil, err)
+	confPassword.Unlock()
+
+	currentPassword, err := confPassword.ReadPassword()
+	assert.Equal(t, nil, err)
 	assert.Equal(t, tempPassword, currentPassword)
 
 	tempPassword = randString(20)
-	err = WritePassword(tempPasswordFile, tempPassword)
-	assert.Equal(t, err, nil)
+	locked, err = anotherConfPassword.GetExLock()
+	defer anotherConfPassword.Unlock()
+	assert.Equal(t, nil, err)
+	assert.True(t, locked)
 
-	currentPassword, err = readPassword(tempPasswordFile)
-	assert.Equal(t, err, nil)
-	assert.NotEqual(t, currentPassword, tempPassword)
+	err = anotherConfPassword.WritePassword(tempPassword)
+	assert.Equal(t, nil, err)
+
+	currentPassword, err = anotherConfPassword.ReadPassword()
+	assert.Equal(t, nil, err)
+	assert.Equal(t, tempPassword, currentPassword)
 
 	err = os.Remove(tempPasswordFile)
 }
 
+func createTestConfig() error {
+	conf := &Config{
+		ServerURL:  apiURL,
+		Group:      defaultGroup,
+		DeviceName: randString(10),
+	}
+	d, err := yaml.Marshal(conf)
+	if err != nil {
+		return err
+	}
+	err = ioutil.WriteFile(testConfig, d, 0600)
+	return err
+}
+
+// runMultipleRegistrations registers supplied count APIs on multiple threads
+// and returns a  channel in which the registered passwords will be supplied
+func runMultipleRegistrations(count int) (int, chan string) {
+	messages := make(chan string)
+
+	for i := 0; i < count; i++ {
+		go func() {
+			api, err := NewAPIFromConfig(testConfig)
+			if err != nil {
+				messages <- err.Error()
+			} else {
+				messages <- api.device.password
+			}
+		}()
+	}
+	return count, messages
+}
+
+func removeTestConfig() {
+	_ = os.Remove(testConfig)
+	_ = os.Remove(privConfigFilename(testConfig))
+}
+
+func TestMultipleRegistrations(t *testing.T) {
+	err := createTestConfig()
+	defer removeTestConfig()
+
+	require.Equal(t, nil, err, "Must be able to make test config "+testConfig)
+	count, passwords := runMultipleRegistrations(4)
+	password := <-passwords
+	for i := 1; i < count; i++ {
+		pass := <-passwords
+		assert.Equal(t, password, pass)
+	}
+}
+
 // getAPI returns a CacophonyAPI for testing purposes using provided url and password with random name
 // if register is set will provide a random token and password and set justRegistered
 func getAPI(url, password string, register bool) *CacophonyAPI {

config.go

@@ -75,9 +75,57 @@ func ParseConfig(buf []byte) (*Config, error) {
 	return conf, nil
 }
 
-//ReadPassword reads the password from supplied filename location
-func ReadPassword(filename string) (string, error) {
-	buf, err := ioutil.ReadFile(filename)
+const (
+	lockfile       = "/var/lock/go-api-config.lock"
+	lockRetryDelay = 678 * time.Millisecond
+	lockTimeout    = 5 * time.Second
+)
+
+type ConfigPassword struct {
+	fileLock *flock.Flock
+	filename string
+	password string
+}
+
+func NewConfigPassword(filename string) *ConfigPassword {
+	return &ConfigPassword{
+		filename: filename,
+		fileLock: flock.New(lockfile),
+	}
+}
+
+func (confPassword *ConfigPassword) Unlock() {
+	confPassword.fileLock.Unlock()
+}
+
+// GetExLock acquires an exclusive lock on confPassword
+func (confPassword *ConfigPassword) GetExLock() (bool, error) {
+	lockCtx, cancel := context.WithTimeout(context.Background(), lockTimeout)
+	defer cancel()
+	locked, err := confPassword.fileLock.TryLockContext(lockCtx, lockRetryDelay)
+	return locked, err
+}
+
+// getReadLock  acquires a read lock on the supplied Flock struct
+func getReadLock(fileLock *flock.Flock) (bool, error) {
+	lockCtx, cancel := context.WithTimeout(context.Background(), lockTimeout)
+	defer cancel()
+	locked, err := fileLock.TryRLockContext(lockCtx, lockRetryDelay)
+	return locked, err
+}
+
+// ReadPassword acquires a readlock and reads the password
+func (confPassword *ConfigPassword) ReadPassword() (string, error) {
+	locked := confPassword.fileLock.Locked()
+	if locked == false {
+		locked, err := getReadLock(confPassword.fileLock)
+		if locked == false || err != nil {
+			return "", err
+		}
+		defer confPassword.Unlock()
+	}
+
+	buf, err := ioutil.ReadFile(confPassword.filename)
 	if os.IsNotExist(err) {
 		return "", nil
 	} else if err != nil {
@@ -90,32 +138,22 @@ func ReadPassword(filename string) (string, error) {
 	return conf.Password, nil
 }
 
-// WritePassword takes a filename and password. Locks the file (OS-level) and
-// writes the supplied password to the file
-func WritePassword(filename, password string) error {
+// WritePassword checks the file is locked and writes the password
+func (confPassword *ConfigPassword) WritePassword(password string) error {
 	conf := PrivateConfig{Password: password}
 	buf, err := yaml.Marshal(&conf)
 	if err != nil {
 		return err
 	}
-
-	fileLock := flock.New(filename)
-	lockCtx, cancel := context.WithTimeout(context.Background(), 3*time.Second)
-	defer cancel()
-	locked, err := fileLock.TryLockContext(lockCtx, 678*time.Millisecond)
-	if err != nil {
-		return err
-	}
-	if locked {
-		err = ioutil.WriteFile(filename, buf, 0600)
-		fileLock.Unlock()
+	if confPassword.fileLock.Locked() {
+		err = ioutil.WriteFile(confPassword.filename, buf, 0600)
 	} else {
-		return fmt.Errorf("WritePassword could not get file lock %v", filename)
+		return fmt.Errorf("WritePassword could not get file lock %v", confPassword.filename)
 	}
 	return err
 }
 
-// privCOnfigFileName take a configFile and creates an associated
+// privConfigFilename take a configFile and creates an associated
 // file to store the password in with suffix -priv.yaml
 func privConfigFilename(configFile string) string {
 	dirname, filename := filepath.Split(configFile)

db-test-seed.sql

@@ -1,10 +1,3 @@
--- sudo -i -u postgres psql cacophonytest -f/home/zaza/go/src/github.com/TheCacophonyProject/go-api/db-seed.sql
---docker cp db-seed.sql cacophony-api:/db-seed.sql
--- sudo -i -u postgres psql cacophonytest -f/db-seed.sql
-
---admin user  admin//password
---INSERT INTO "Users" (username, email, password, "globalPermission", "createdAt", "updatedAt") VALUES ('admin_test', '[email protected]', '$2a$10$S..GUBx1zVb6r1QklWu5kOgx5czlAPJM1JxeJ0uHAO3nRRhDtNazm', 'write', now(), now());
-
  --test-group
 INSERT INTO "Groups" ("id","groupname","createdAt","updatedAt") VALUES (DEFAULT,'test-group','2019-03-14 20:15:23.423 +00:00','2019-03-14 20:15:23.423 +00:00');