Improve error handling in list operations

Author: bootjp

adapter/dynamodb_transcoder.go

@@ -87,8 +87,8 @@ func (t *dynamodbTranscoder) valueAttrToOps(key []byte, val attributeValue) (*kv
 	if len(val.L) > 0 {
 		var elems []*kv.Elem[kv.OP]
 		for i, item := range val.L {
-			if item.S == "" {
-				return nil, errors.New("only string list items are supported")
+			if len(item.L) > 0 {
+				return nil, errors.New("nested lists are not supported")
 			}
 			elems = append(elems, &kv.Elem[kv.OP]{
 				Op:    kv.Put,
@@ -110,8 +110,8 @@ func (t *dynamodbTranscoder) valueAttrToOps(key []byte, val attributeValue) (*kv
 		return &kv.OperationGroup[kv.OP]{IsTxn: true, Elems: elems}, nil
 	}
 
-	// Default: simple string
-	if val.S == "" {
+	// Default: simple string (allow empty string). Reject only when both S and L are absent.
+	if val.S == "" && len(val.L) == 0 {
 		return nil, errors.New("unsupported attribute type (only S or L of S)")
 	}
 	return &kv.OperationGroup[kv.OP]{

adapter/redis.go

@@ -183,11 +183,8 @@ func (r *RedisServer) set(conn redcon.Conn, cmd redcon.Command) {
 		return
 	}
 	if isList {
-		// delete list metadata so this becomes a plain string key
-		if err := r.deleteList(context.Background(), cmd.Args[1]); err != nil {
-			conn.WriteError(err.Error())
-			return
-		}
+		conn.WriteError("WRONGTYPE Operation against a key holding the wrong kind of value")
+		return
 	}
 
 	res, err := r.redisTranscoder.SetToRequest(cmd.Args[1], cmd.Args[2])
@@ -629,15 +626,15 @@ func (t *txnContext) applyLRange(cmd redcon.Command) (redisResult, error) {
 }
 
 func parseRangeBounds(startRaw, endRaw []byte, total int) (int, int, error) {
-	start, err := strconv.Atoi(string(startRaw))
+	start, err := parseInt(startRaw)
 	if err != nil {
-		return 0, 0, errors.WithStack(err)
+		return 0, 0, err
 	}
-	end, err := strconv.Atoi(string(endRaw))
+	end, err := parseInt(endRaw)
 	if err != nil {
-		return 0, 0, errors.WithStack(err)
+		return 0, 0, err
 	}
-	s, e := clampRange(start, end, total)
+	s, e := clampRange(int(start), int(end), total)
 	return s, e, nil
 }
 
@@ -723,6 +720,10 @@ func (t *txnContext) buildListElems() ([]*kv.Elem[kv.OP], error) {
 		userKey := []byte(k)
 
 		if st.deleted {
+			// delete all persisted list items
+			for seq := st.meta.Head; seq < st.meta.Tail; seq++ {
+				elems = append(elems, &kv.Elem[kv.OP]{Op: kv.Del, Key: listItemKey(userKey, seq)})
+			}
 			elems = append(elems, &kv.Elem[kv.OP]{Op: kv.Del, Key: listMetaKey(userKey)})
 			continue
 		}
@@ -884,17 +885,32 @@ func (r *RedisServer) listRPush(ctx context.Context, key []byte, values [][]byte
 }
 
 func (r *RedisServer) deleteList(ctx context.Context, key []byte) error {
-	_, exists, err := r.loadListMeta(ctx, key)
+	meta, exists, err := r.loadListMeta(ctx, key)
 	if err != nil {
 		return err
 	}
 	if !exists {
 		return nil
 	}
 
-	ops := []*kv.Elem[kv.OP]{
-		{Op: kv.Del, Key: listMetaKey(key)},
+	start := listItemKey(key, math.MinInt64)
+	end := listItemKey(key, math.MaxInt64)
+
+	kvs, err := r.store.Scan(ctx, start, end, math.MaxInt)
+	if err != nil {
+		return errors.WithStack(err)
+	}
+
+	ops := make([]*kv.Elem[kv.OP], 0, len(kvs)+1)
+	for _, kvp := range kvs {
+		ops = append(ops, &kv.Elem[kv.OP]{Op: kv.Del, Key: kvp.Key})
 	}
+	// delete meta last
+	ops = append(ops, &kv.Elem[kv.OP]{Op: kv.Del, Key: listMetaKey(key)})
+
+	// ensure meta bounds consistent even if scan missed (in case of empty list)
+	_ = meta
+
 	group := &kv.OperationGroup[kv.OP]{IsTxn: true, Elems: ops}
 	_, err = r.coordinator.Dispatch(group)
 	return errors.WithStack(err)
@@ -966,7 +982,16 @@ func (r *RedisServer) proxyLRange(key []byte, startRaw, endRaw []byte) ([]string
 	cli := redis.NewClient(&redis.Options{Addr: leaderAddr})
 	defer func() { _ = cli.Close() }()
 
-	res, err := cli.LRange(context.Background(), string(key), parseInt(startRaw), parseInt(endRaw)).Result()
+	start, err := parseInt(startRaw)
+	if err != nil {
+		return nil, err
+	}
+	end, err := parseInt(endRaw)
+	if err != nil {
+		return nil, err
+	}
+
+	res, err := cli.LRange(context.Background(), string(key), start, end).Result()
 	return res, errors.WithStack(err)
 }
 
@@ -992,9 +1017,9 @@ func (r *RedisServer) proxyRPush(key []byte, values [][]byte) (int64, error) {
 	return res, errors.WithStack(err)
 }
 
-func parseInt(b []byte) int64 {
-	i, _ := strconv.ParseInt(string(b), 10, 64)
-	return i
+func parseInt(b []byte) (int64, error) {
+	i, err := strconv.ParseInt(string(b), 10, 64)
+	return i, errors.WithStack(err)
 }
 
 // tryLeaderGet proxies a GET to the current Raft leader, returning the value and

adapter/test_util.go

@@ -10,8 +10,6 @@ import (
 	"testing"
 	"time"
 
-	"golang.org/x/sys/unix"
-
 	"github.com/Jille/raft-grpc-leader-rpc/leaderhealth"
 	transport "github.com/Jille/raft-grpc-transport"
 	"github.com/Jille/raftadmin"
@@ -23,6 +21,7 @@ import (
 	"github.com/hashicorp/raft"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"golang.org/x/sys/unix"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/credentials/insecure"
 )

store/list_store.go

@@ -3,8 +3,8 @@ package store
 import (
 	"bytes"
 	"context"
+	"encoding/hex"
 	"encoding/json"
-	"fmt"
 	"math"
 
 	"github.com/cockroachdb/errors"
@@ -220,7 +220,32 @@ func ListMetaKey(userKey []byte) []byte {
 
 // ListItemKey builds the item key for a user key and sequence number.
 func ListItemKey(userKey []byte, seq int64) []byte {
-	return []byte(fmt.Sprintf("%s%s|%0*d", ListItemPrefix, userKey, ListSeqWidth, seq))
+	// Offset sign bit (seq ^ minInt64) to preserve order, then big-endian encode and hex.
+	var raw [8]byte
+	encodeSortableInt64(raw[:], seq)
+	hexSeq := make([]byte, hex.EncodedLen(len(raw)))
+	hex.Encode(hexSeq, raw[:])
+
+	buf := make([]byte, 0, len(ListItemPrefix)+len(userKey)+1+len(hexSeq))
+	buf = append(buf, ListItemPrefix...)
+	buf = append(buf, userKey...)
+	buf = append(buf, '|')
+	buf = append(buf, hexSeq...)
+	return buf
+}
+
+// encodeSortableInt64 writes seq with sign bit flipped (seq ^ minInt64) in big-endian order.
+const sortableInt64Bytes = 8
+
+func encodeSortableInt64(dst []byte, seq int64) {
+	if len(dst) < sortableInt64Bytes {
+		return
+	}
+	sortable := seq ^ math.MinInt64
+	for i := sortableInt64Bytes - 1; i >= 0; i-- {
+		dst[i] = byte(sortable)
+		sortable >>= 8
+	}
 }
 
 func clampRange(start, end, length int) (int, int) {
@@ -263,7 +288,7 @@ func ExtractListUserKey(key []byte) []byte {
 		trimmed := bytes.TrimPrefix(key, []byte(ListItemPrefix))
 		idx := bytes.LastIndexByte(trimmed, '|')
 		if idx == -1 {
-			return trimmed
+			return nil
 		}
 		return trimmed[:idx]
 	default: