Merge pull request #43 from EchoVault/chore/echovault-test

Add test coverage for data replication layer

.github/workflows/go.yml

@@ -27,7 +27,7 @@ jobs:
       run: go build -v ./cmd/main.go
 
     - name: Test
-      run: make test-unit
+      run: make test
 
     - name: Test for Data Race
       run: make test-race

Makefile

@@ -17,7 +17,7 @@ run:
 	make build && \
 	docker-compose up --build
 
-test-unit:
+test:
 	env RACE=false OUT=internal/modules/admin/testdata make build-modules-test && \
 	env RACE=false OUT=echovault/testdata make build-modules-test && \
 	go clean -testcache && \

docker-compose.yaml

@@ -63,9 +63,9 @@ services:
       - RAFT_PORT=8000
       - ML_PORT=7946
       - SERVER_ID=1
-      - JOIN_ADDR=cluster_node_2:7946
+      - JOIN_ADDR=2/cluster_node_2:7946
       - DATA_DIR=/var/lib/echovault
-      - IN_MEMORY=false
+      - IN_MEMORY=true
       - TLS=false
       - MTLS=false
       - BOOTSTRAP_CLUSTER=true
@@ -110,9 +110,9 @@ services:
       - RAFT_PORT=8000
       - ML_PORT=7946
       - SERVER_ID=2
-      - JOIN_ADDR=cluster_node_3:7946
+      - JOIN_ADDR=3/cluster_node_3:7946
       - DATA_DIR=/var/lib/echovault
-      - IN_MEMORY=false
+      - IN_MEMORY=true
       - TLS=false
       - MTLS=false
       - BOOTSTRAP_CLUSTER=false
@@ -157,9 +157,9 @@ services:
       - RAFT_PORT=8000
       - ML_PORT=7946
       - SERVER_ID=3
-      - JOIN_ADDR=cluster_node_4:7946
+      - JOIN_ADDR=4/cluster_node_4:7946
       - DATA_DIR=/var/lib/echovault
-      - IN_MEMORY=false
+      - IN_MEMORY=true
       - TLS=false
       - MTLS=false
       - BOOTSTRAP_CLUSTER=false
@@ -204,9 +204,9 @@ services:
       - RAFT_PORT=8000
       - ML_PORT=7946
       - SERVER_ID=4
-      - JOIN_ADDR=cluster_node_5:7946
+      - JOIN_ADDR=5/cluster_node_5:7946
       - DATA_DIR=/var/lib/echovault
-      - IN_MEMORY=false
+      - IN_MEMORY=true
       - TLS=false
       - MTLS=false
       - BOOTSTRAP_CLUSTER=false
@@ -251,9 +251,9 @@ services:
       - RAFT_PORT=8000
       - ML_PORT=7946
       - SERVER_ID=5
-      - JOIN_ADDR=cluster_node_1:7946
+      - JOIN_ADDR=1/cluster_node_1:7946
       - DATA_DIR=/var/lib/echovault
-      - IN_MEMORY=false
+      - IN_MEMORY=true
       - TLS=false
       - MTLS=false
       - BOOTSTRAP_CLUSTER=false

echovault/api_admin.go

@@ -57,42 +57,18 @@ type CommandHandlerFunc func(params CommandHandlerFuncParams) ([]byte, error)
 //
 // Command is the string slice command containing the command that triggered this handler.
 //
-// KeyExists returns true if the key passed to it exists in the store.
+// KeysExist returns a map that specifies whether the key exists in the store.
 //
-// CreateKeyAndLock creates the new key and immediately write locks it. If the key already exists, then
-// it is simply write locked which makes this function safe to call even if the key already exists. Always call
-// KeyUnlock when done after CreateKeyAndLock.
+// GetValues returns a map with the values held at the specified keys. When a key does not exist in the store the
+// associated value will be nil.
 //
-// KeyLock acquires a write lock for the specified key. If the lock is successfully acquired, the function will return
-// (true, nil). Otherwise, it will return false and an error describing why the locking failed. Always call KeyUnlock
-// when done after KeyLock.
-//
-// KeyUnlock releases the write lock for the specified key. Always call this after KeyLock otherwise the key will not be
-// lockable by any future invocations of this command or other commands.
-//
-// KeyRLock acquires a read lock for the specified key. If the lock is successfully acquired, the function will return
-// (true, nil). Otherwise, it will return false and an error describing why the locking failed. Always call KeyRUnlock
-// when done after KeyRLock.
-//
-// KeyRUnlock releases the real lock for the specified key. Always call this after KeyRLock otherwise the key will not be
-// write-lockable by any future invocations of this command or other commands.
-//
-// GetValue returns the value held at the specified key as an interface{}. Make sure to invoke KeyLock or KeyRLock on the
-// key before GetValue to ensure thread safety.
-//
-// SetValue sets the value at the specified key. Make sure to invoke KeyLock on the key before
-// SetValue to ensure thread safety.
+// SetValues sets the keys given with their associated values.
 type CommandHandlerFuncParams struct {
-	Context          context.Context
-	Command          []string
-	KeyExists        func(ctx context.Context, key string) bool
-	CreateKeyAndLock func(ctx context.Context, key string) (bool, error)
-	KeyLock          func(ctx context.Context, key string) (bool, error)
-	KeyUnlock        func(ctx context.Context, key string)
-	KeyRLock         func(ctx context.Context, key string) (bool, error)
-	KeyRUnlock       func(ctx context.Context, key string)
-	GetValue         func(ctx context.Context, key string) interface{}
-	SetValue         func(ctx context.Context, key string, value interface{}) error
+	Context   context.Context
+	Command   []string
+	KeysExist func(keys []string) map[string]bool
+	GetValues func(ctx context.Context, keys []string) map[string]interface{}
+	SetValues func(ctx context.Context, entries map[string]interface{}) error
 }
 
 // CommandOptions provides the specification of the command to be added to the EchoVault instance.
@@ -268,16 +244,11 @@ func (server *EchoVault) AddCommand(command CommandOptions) error {
 			}),
 			HandlerFunc: internal.HandlerFunc(func(params internal.HandlerFuncParams) ([]byte, error) {
 				return command.HandlerFunc(CommandHandlerFuncParams{
-					Context:          params.Context,
-					Command:          params.Command,
-					KeyLock:          params.KeyLock,
-					KeyUnlock:        params.KeyUnlock,
-					KeyRLock:         params.KeyRLock,
-					KeyRUnlock:       params.KeyRUnlock,
-					KeyExists:        params.KeyExists,
-					CreateKeyAndLock: params.CreateKeyAndLock,
-					GetValue:         params.GetValue,
-					SetValue:         params.SetValue,
+					Context:   params.Context,
+					Command:   params.Command,
+					KeysExist: params.KeysExist,
+					GetValues: params.GetValues,
+					SetValues: params.SetValues,
 				})
 			}),
 		})
@@ -338,16 +309,11 @@ func (server *EchoVault) AddCommand(command CommandOptions) error {
 			}),
 			HandlerFunc: internal.HandlerFunc(func(params internal.HandlerFuncParams) ([]byte, error) {
 				return sc.HandlerFunc(CommandHandlerFuncParams{
-					Context:          params.Context,
-					Command:          params.Command,
-					KeyLock:          params.KeyLock,
-					KeyUnlock:        params.KeyUnlock,
-					KeyRLock:         params.KeyRLock,
-					KeyRUnlock:       params.KeyRUnlock,
-					KeyExists:        params.KeyExists,
-					CreateKeyAndLock: params.CreateKeyAndLock,
-					GetValue:         params.GetValue,
-					SetValue:         params.SetValue,
+					Context:   params.Context,
+					Command:   params.Command,
+					KeysExist: params.KeysExist,
+					GetValues: params.GetValues,
+					SetValues: params.SetValues,
 				})
 			}),
 		}

echovault/api_admin_test.go

@@ -243,7 +243,6 @@ func TestEchoVault_ExecuteCommand(t *testing.T) {
 			r := resp.NewReader(bytes.NewReader(b))
 			v, _, _ := r.ReadValue()
 			if v.Integer() != tt.wantRes {
-				fmt.Println("RES: ", string(b))
 				t.Errorf("ExecuteCommand() response = %d, wantRes %d", v.Integer(), tt.wantRes)
 			}
 		})

echovault/api_hash_test.go

@@ -616,13 +616,13 @@ func TestEchoVault_HSET(t *testing.T) {
 			wantErr:         false,
 		},
 		{
-			name:            "HSET returns error when the target key is not a map",
+			name:            "HSET overwrites when the target key is not a map",
 			key:             "key6",
 			presetValue:     "Default preset value",
 			fieldValuePairs: map[string]string{"field1": "value1"},
 			hsetFunc:        server.HSet,
-			want:            0,
-			wantErr:         true,
+			want:            1,
+			wantErr:         false,
 		},
 	}
 	for _, tt := range tests {

echovault/api_list.go

@@ -15,7 +15,6 @@
 package echovault
 
 import (
-	"fmt"
 	"github.com/echovault/echovault/internal"
 	"strconv"
 	"strings"
@@ -34,7 +33,6 @@ import (
 // "LLen command on non-list item" - when the provided key exists but is not a list.
 func (server *EchoVault) LLen(key string) (int, error) {
 	b, err := server.handleCommand(server.context, internal.EncodeCommand([]string{"LLEN", key}), nil, false, true)
-	fmt.Println(key, string(b), err)
 	if err != nil {
 		return 0, err
 	}

echovault/api_set_test.go

@@ -376,7 +376,7 @@ func TestEchoVault_SINTER(t *testing.T) {
 				"key12": set.NewSet([]string{"one", "two", "thirty-six", "twelve", "eleven"}),
 				"key13": set.NewSet([]string{"seven", "eight", "nine", "ten", "twelve"}),
 			},
-			keys:    []string{"non-existent", "key7", "key8"},
+			keys:    []string{"non-existent", "key12", "key13"},
 			want:    []string{},
 			wantErr: false,
 		},
@@ -467,13 +467,12 @@ func TestEchoVault_SINTERCARD(t *testing.T) {
 			wantErr: false,
 		},
 		{
-			name: "Return 0 if any of the keys does not exist",
+			name: "Return error if any of the keys is non-existent",
 			presetValues: map[string]interface{}{
 				"key11": set.NewSet([]string{"one", "two", "three", "four", "five", "six", "seven", "eight"}),
-				"key12": "Default value",
 				"key13": set.NewSet([]string{"one"}),
 			},
-			keys:    []string{"key11", "key12", "key13", "non-existent"},
+			keys:    []string{"key11", "key12", "key13"},
 			limit:   0,
 			want:    0,
 			wantErr: false,
@@ -578,7 +577,7 @@ func TestEchoVault_SINTERSTORE(t *testing.T) {
 				"key12": set.NewSet([]string{"one", "two", "thirty-six", "twelve", "eleven"}),
 				"key13": set.NewSet([]string{"seven", "eight", "nine", "ten", "twelve"}),
 			},
-			keys:    []string{"non-existent", "key7", "key8"},
+			keys:    []string{"non-existent", "key12", "key13"},
 			want:    0,
 			wantErr: false,
 		},

echovault/echovault.go

@@ -61,9 +61,8 @@ type EchoVault struct {
 	// the new number is the new connection's ID.
 	connId atomic.Uint64
 
-	store           map[string]internal.KeyData // Data store to hold the keys and their associated data, expiry time, etc.
-	keyLocks        map[string]*sync.RWMutex    // Map to hold all the individual key locks.
-	keyCreationLock *sync.Mutex                 // The mutex for creating a new key. Only one goroutine should be able to create a key at a time.
+	storeLock *sync.RWMutex               // Global read-write mutex for entire store.
+	store     map[string]internal.KeyData // Data store to hold the keys and their associated data, expiry time, etc.
 
 	// Holds all the keys that are currently associated with an expiry.
 	keysWithExpiry struct {
@@ -128,21 +127,20 @@ func WithConfig(config config.Config) func(echovault *EchoVault) {
 // This functions accepts the WithContext, WithConfig and WithCommands options.
 func NewEchoVault(options ...func(echovault *EchoVault)) (*EchoVault, error) {
 	echovault := &EchoVault{
-		clock:           clock.NewClock(),
-		context:         context.Background(),
-		config:          config.DefaultConfig(),
-		store:           make(map[string]internal.KeyData),
-		keyLocks:        make(map[string]*sync.RWMutex),
-		keyCreationLock: &sync.Mutex{},
-		commandsRWMut:   sync.RWMutex{},
+		clock:         clock.NewClock(),
+		context:       context.Background(),
+		config:        config.DefaultConfig(),
+		storeLock:     &sync.RWMutex{},
+		store:         make(map[string]internal.KeyData),
+		commandsRWMut: sync.RWMutex{},
 		commands: func() []internal.Command {
 			var commands []internal.Command
 			commands = append(commands, acl.Commands()...)
 			commands = append(commands, admin.Commands()...)
+			commands = append(commands, connection.Commands()...)
 			commands = append(commands, generic.Commands()...)
 			commands = append(commands, hash.Commands()...)
 			commands = append(commands, list.Commands()...)
-			commands = append(commands, connection.Commands()...)
 			commands = append(commands, pubsub.Commands()...)
 			commands = append(commands, set.Commands()...)
 			commands = append(commands, sorted_set.Commands()...)
@@ -195,15 +193,17 @@ func NewEchoVault(options ...func(echovault *EchoVault)) (*EchoVault, error) {
 		echovault.raft = raft.NewRaft(raft.Opts{
 			Config:                echovault.config,
 			GetCommand:            echovault.getCommand,
-			CreateKeyAndLock:      echovault.CreateKeyAndLock,
-			SetValue:              echovault.SetValue,
-			SetExpiry:             echovault.SetExpiry,
-			KeyUnlock:             echovault.KeyUnlock,
-			DeleteKey:             echovault.DeleteKey,
+			SetValues:             echovault.setValues,
+			SetExpiry:             echovault.setExpiry,
 			StartSnapshot:         echovault.startSnapshot,
 			FinishSnapshot:        echovault.finishSnapshot,
 			SetLatestSnapshotTime: echovault.setLatestSnapshot,
 			GetHandlerFuncParams:  echovault.getHandlerFuncParams,
+			DeleteKey: func(key string) error {
+				echovault.storeLock.Lock()
+				defer echovault.storeLock.Unlock()
+				return echovault.deleteKey(key)
+			},
 			GetState: func() map[string]internal.KeyData {
 				state := make(map[string]internal.KeyData)
 				for k, v := range echovault.getState() {
@@ -245,14 +245,10 @@ func NewEchoVault(options ...func(echovault *EchoVault)) (*EchoVault, error) {
 			}),
 			snapshot.WithSetKeyDataFunc(func(key string, data internal.KeyData) {
 				ctx := context.Background()
-				if _, err := echovault.CreateKeyAndLock(ctx, key); err != nil {
+				if err := echovault.setValues(ctx, map[string]interface{}{key: data.Value}); err != nil {
 					log.Println(err)
 				}
-				if err := echovault.SetValue(ctx, key, data.Value); err != nil {
-					log.Println(err)
-				}
-				echovault.SetExpiry(ctx, key, data.ExpireAt, false)
-				echovault.KeyUnlock(ctx, key)
+				echovault.setExpiry(ctx, key, data.ExpireAt, false)
 			}),
 		)
 		// Set up standalone AOF engine
@@ -273,14 +269,10 @@ func NewEchoVault(options ...func(echovault *EchoVault)) (*EchoVault, error) {
 			}),
 			aof.WithSetKeyDataFunc(func(key string, value internal.KeyData) {
 				ctx := context.Background()
-				if _, err := echovault.CreateKeyAndLock(ctx, key); err != nil {
-					log.Println(err)
-				}
-				if err := echovault.SetValue(ctx, key, value.Value); err != nil {
+				if err := echovault.setValues(ctx, map[string]interface{}{key: value.Value}); err != nil {
 					log.Println(err)
 				}
-				echovault.SetExpiry(ctx, key, value.ExpireAt, false)
-				echovault.KeyUnlock(ctx, key)
+				echovault.setExpiry(ctx, key, value.ExpireAt, false)
 			}),
 			aof.WithHandleCommandFunc(func(command []byte) {
 				_, err := echovault.handleCommand(context.Background(), command, nil, true, false)