Skip to content

Commit 8679ec3

Browse files
derekperkinsburaksezer
authored andcommitted
fix pipeline Get & GetPut panic on missing keys
(cherry picked from commit 0ce1e6a)
1 parent 93c06b6 commit 8679ec3

2 files changed

Lines changed: 75 additions & 3 deletions

File tree

pipeline.go

Lines changed: 34 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"bytes"
1919
"context"
2020
"errors"
21+
"fmt"
2122
"runtime"
2223
"strconv"
2324
"sync"
@@ -164,8 +165,15 @@ func (f *FutureGet) Result() (*GetResponse, error) {
164165
if cmd.Err() != nil {
165166
return nil, processProtocolError(cmd.Err())
166167
}
168+
value, nilValue, err := pipelineStringValue(cmd)
169+
if err != nil {
170+
return nil, err
171+
}
172+
if nilValue {
173+
return nil, ErrKeyNotFound
174+
}
167175
stringCmd := redis.NewStringCmd(context.Background(), cmd.Args()...)
168-
stringCmd.SetVal(cmd.(*redis.Cmd).Val().(string))
176+
stringCmd.SetVal(value)
169177
return f.dp.dm.makeGetResponse(stringCmd)
170178
default:
171179
return nil, ErrNotReady
@@ -392,8 +400,15 @@ func (f *FutureGetPut) Result() (*GetResponse, error) {
392400
if cmd.Err() != nil {
393401
return nil, processProtocolError(cmd.Err())
394402
}
403+
value, nilValue, err := pipelineStringValue(cmd)
404+
if err != nil {
405+
return nil, err
406+
}
407+
if nilValue {
408+
return nil, nil
409+
}
395410
stringCmd := redis.NewStringCmd(context.Background(), cmd.Args()...)
396-
stringCmd.SetVal(cmd.(*redis.Cmd).Val().(string))
411+
stringCmd.SetVal(value)
397412
return f.dp.dm.makeGetResponse(stringCmd)
398413
default:
399414
return nil, ErrNotReady
@@ -645,3 +660,20 @@ func putPipelineCmdsIntoPool(cmds []redis.Cmder) {
645660
cmds = cmds[:0]
646661
pipelineCmdPool.Put(cmds)
647662
}
663+
664+
func pipelineStringValue(cmd redis.Cmder) (string, bool, error) {
665+
redisCmd, ok := cmd.(*redis.Cmd)
666+
if !ok {
667+
return "", false, fmt.Errorf("unexpected pipeline command type %T", cmd)
668+
}
669+
670+
value := redisCmd.Val()
671+
if value == nil {
672+
return "", true, nil
673+
}
674+
stringValue, ok := value.(string)
675+
if !ok {
676+
return "", false, fmt.Errorf("unexpected pipeline command value type %T", value)
677+
}
678+
return stringValue, false, nil
679+
}

pipeline_test.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ import (
2020
"testing"
2121
"time"
2222

23-
"github.com/olric-data/olric/internal/testutil"
23+
"github.com/redis/go-redis/v9"
2424
"github.com/stretchr/testify/require"
25+
26+
"github.com/olric-data/olric/internal/testutil"
2527
)
2628

2729
func TestDMapPipeline_Put(t *testing.T) {
@@ -107,6 +109,25 @@ func TestDMapPipeline_Get(t *testing.T) {
107109
}
108110
}
109111

112+
func TestFutureGet_Result_NilPipelineValue(t *testing.T) {
113+
cmd := redis.NewCmd(context.Background(), "dm.get", "mydmap", "missing", "RW")
114+
cmd.SetVal(nil)
115+
116+
ctx, cancel := context.WithCancel(context.Background())
117+
cancel()
118+
future := &FutureGet{
119+
dp: &DMapPipeline{
120+
result: map[uint64][]redis.Cmder{0: {cmd}},
121+
},
122+
ctx: ctx,
123+
closedCtx: context.Background(),
124+
}
125+
126+
res, err := future.Result()
127+
require.Nil(t, res)
128+
require.ErrorIs(t, err, ErrKeyNotFound)
129+
}
130+
110131
func TestDMapPipeline_Delete(t *testing.T) {
111132
cluster := newTestOlricCluster(t)
112133
db := cluster.addMember(t)
@@ -293,6 +314,25 @@ func TestDMapPipeline_GetPut(t *testing.T) {
293314
}
294315
}
295316

317+
func TestFutureGetPut_Result_NilPipelineValue(t *testing.T) {
318+
cmd := redis.NewCmd(context.Background(), "dm.getput", "mydmap", "key", "value", "RW")
319+
cmd.SetVal(nil)
320+
321+
ctx, cancel := context.WithCancel(context.Background())
322+
cancel()
323+
future := &FutureGetPut{
324+
dp: &DMapPipeline{
325+
result: map[uint64][]redis.Cmder{0: {cmd}},
326+
},
327+
ctx: ctx,
328+
closedCtx: context.Background(),
329+
}
330+
331+
res, err := future.Result()
332+
require.NoError(t, err)
333+
require.Nil(t, res)
334+
}
335+
296336
func TestDMapPipeline_IncrByFloat(t *testing.T) {
297337
cluster := newTestOlricCluster(t)
298338
db := cluster.addMember(t)

0 commit comments

Comments
 (0)