Skip to content

Commit

Permalink
refeactor: remove command name check at each command level
Browse files Browse the repository at this point in the history
There is no need to check the command name at this level because
we are garanteed to call from on the right command type in the code.
  • Loading branch information
ynachi committed Jan 6, 2024
1 parent 6defe7f commit 0dcf010
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 21 deletions.
13 changes: 12 additions & 1 deletion command/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,13 @@ func NewCommand(cmdName string) Command {
}
}

// use that to fail fast when the command is not valid.
var registeredCommands = map[string]struct{}{
"ping": {},
"set": {},
"get": {},
}

// GetCmdName gets a command name from a Frame Array.
func GetCmdName(f *frame.Array) (string, error) {
if f.Size() < 1 {
Expand All @@ -47,5 +54,9 @@ func GetCmdName(f *frame.Array) (string, error) {
if !ok {
return "", gerror.ErrNotGcacheCmd
}
return strings.ToLower(cmdName.Value()), nil
cmdNameValue := strings.ToLower(cmdName.Value())
if _, ok = registeredCommands[cmdNameValue]; !ok {
return "", gerror.ErrInvalidCmdName
}
return cmdNameValue, nil
}
6 changes: 1 addition & 5 deletions command/get.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,7 @@ func (c *Get) Apply(cache *db.Cache, dest *bufio.Writer) {
}

func (c *Get) FromFrame(f *frame.Array) error {
cmdName, err := GetCmdName(f)
if err != nil {
return err
}
if cmdName != "get" || f.Size() != 2 {
if f.Size() != 2 {
return gerror.ErrInvalidCmdArgs
}
c.key = f.Get(1).(*frame.BulkString).Value()
Expand Down
6 changes: 1 addition & 5 deletions command/ping.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,8 @@ func (c *Ping) Apply(_ *db.Cache, dest *bufio.Writer) {
}

func (c *Ping) FromFrame(f *frame.Array) error {
cmdName, err := GetCmdName(f)
if err != nil {
return err
}
switch {
case cmdName != "ping" || f.Size() > 2:
case f.Size() > 2:
return gerror.ErrInvalidPingCommand
case f.Size() == 1:
c.message = "PONG"
Expand Down
7 changes: 2 additions & 5 deletions command/ping_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"bytes"
"github.com/stretchr/testify/assert"
"github.com/ynachi/gcache/frame"
"github.com/ynachi/gcache/gerror"
"log/slog"
"strings"
"testing"
Expand Down Expand Up @@ -53,9 +54,6 @@ func TestPing_FromFrame(t *testing.T) {
_ = pingWithMsg.Append(frame.NewBulkString("PING"))
_ = pingWithMsg.Append(frame.NewBulkString("Hello World"))

wrongCmd := frame.NewArray(1)
_ = wrongCmd.Append(frame.NewBulkString("PINg"))

pingTooManyArgs := frame.NewArray(3)
_ = pingTooManyArgs.Append(frame.NewBulkString("PING"))
_ = pingTooManyArgs.Append(frame.NewBulkString("Hello World"))
Expand All @@ -69,8 +67,7 @@ func TestPing_FromFrame(t *testing.T) {
}{
{name: "ValidSimplePing", frame: simplePING, want: "PONG", wantError: nil},
{name: "ValidPingWithMessage", frame: pingWithMsg, want: "Hello World", wantError: nil},
{name: "InvalidWrongCmdWord", frame: wrongCmd, want: "", wantError: ErrInvalidCmdName},
{name: "InvalidTooManyArgs", frame: pingTooManyArgs, want: "", wantError: ErrInvalidPingCommand},
{name: "InvalidTooManyArgs", frame: pingTooManyArgs, want: "", wantError: gerror.ErrInvalidPingCommand},
}

for _, tt := range tests {
Expand Down
6 changes: 1 addition & 5 deletions command/set.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,7 @@ func (c *Set) Apply(cache *db.Cache, dest *bufio.Writer) {

// @TODO Minimal implementation for now, to fix

Check failure on line 32 in command/set.go

View workflow job for this annotation

GitHub Actions / lint and test

Comment should end in a period (godot)
func (c *Set) FromFrame(f *frame.Array) error {
cmdName, err := GetCmdName(f)
if err != nil {
return err
}
if cmdName != "set" || f.Size() != 3 {
if f.Size() != 3 {
return gerror.ErrInvalidCmdArgs
}
c.key = f.Get(1).(*frame.BulkString).Value()
Expand Down

0 comments on commit 0dcf010

Please sign in to comment.