From 659160289cc6b7242f97de975c03207c3a152c38 Mon Sep 17 00:00:00 2001
From: tidwall <joshbaker77@gmail.com>
Date: Mon, 26 Sep 2022 17:58:51 -0700
Subject: [PATCH] Better READONLY tests

---
 internal/server/readonly.go | 42 +++++++++++++++++++++----------------
 internal/server/server.go   |  2 +-
 tests/aof_test.go           | 13 ++++++++++++
 3 files changed, 38 insertions(+), 19 deletions(-)

diff --git a/internal/server/readonly.go b/internal/server/readonly.go
index 586a9c0c..94067289 100644
--- a/internal/server/readonly.go
+++ b/internal/server/readonly.go
@@ -1,44 +1,50 @@
 package server
 
 import (
-	"strings"
 	"time"
 
 	"github.com/tidwall/resp"
 	"github.com/tidwall/tile38/internal/log"
 )
 
-func (s *Server) cmdReadOnly(msg *Message) (res resp.Value, err error) {
+// READONLY yes|no
+func (s *Server) cmdREADONLY(msg *Message) (resp.Value, error) {
 	start := time.Now()
-	vs := msg.Args[1:]
-	var arg string
-	var ok bool
 
-	if vs, arg, ok = tokenval(vs); !ok || arg == "" {
-		return NOMessage, errInvalidNumberOfArguments
+	// >> Args
+
+	args := msg.Args
+	if len(args) != 2 {
+		return retrerr(errInvalidNumberOfArguments)
 	}
-	if len(vs) != 0 {
-		return NOMessage, errInvalidNumberOfArguments
-	}
-	update := false
-	switch strings.ToLower(arg) {
+
+	switch args[1] {
+	case "yes", "no":
 	default:
-		return NOMessage, errInvalidArgument(arg)
-	case "yes":
+		return retrerr(errInvalidArgument(args[1]))
+	}
+
+	// >> Operation
+
+	var updated bool
+	if args[1] == "yes" {
 		if !s.config.readOnly() {
-			update = true
+			updated = true
 			s.config.setReadOnly(true)
 			log.Info("read only")
 		}
-	case "no":
+	} else {
 		if s.config.readOnly() {
-			update = true
+			updated = true
 			s.config.setReadOnly(false)
 			log.Info("read write")
 		}
 	}
-	if update {
+	if updated {
 		s.config.write(false)
 	}
+
+	// >> Response
+
 	return OKMessage(msg, start), nil
 }
diff --git a/internal/server/server.go b/internal/server/server.go
index ea49da26..9a2770a2 100644
--- a/internal/server/server.go
+++ b/internal/server/server.go
@@ -1181,7 +1181,7 @@ func (s *Server) command(msg *Message, client *Client) (
 	case "replconf":
 		res, err = s.cmdReplConf(msg, client)
 	case "readonly":
-		res, err = s.cmdReadOnly(msg)
+		res, err = s.cmdREADONLY(msg)
 	case "stats":
 		res, err = s.cmdSTATS(msg)
 	case "server":
diff --git a/tests/aof_test.go b/tests/aof_test.go
index 7a68058a..d82667c2 100644
--- a/tests/aof_test.go
+++ b/tests/aof_test.go
@@ -20,6 +20,7 @@ func subTestAOF(g *testGroup) {
 	g.regSubTest("AOF", aof_AOF_test)
 	g.regSubTest("AOFMD5", aof_AOFMD5_test)
 	g.regSubTest("AOFSHRINK", aof_AOFSHRINK_test)
+	g.regSubTest("READONLY", aof_READONLY_test)
 }
 
 func loadAOFAndClose(aof any) error {
@@ -273,3 +274,15 @@ func aof_AOFSHRINK_test(mc *mockServer) error {
 	}
 	return err
 }
+
+func aof_READONLY_test(mc *mockServer) error {
+	return mc.DoBatch(
+		Do("SET", "mykey", "myid", "POINT", "10", "10").OK(),
+		Do("READONLY", "yes").OK(),
+		Do("SET", "mykey", "myid", "POINT", "10", "10").Err("read only"),
+		Do("READONLY", "no").OK(),
+		Do("SET", "mykey", "myid", "POINT", "10", "10").OK(),
+		Do("READONLY").Err("wrong number of arguments for 'readonly' command"),
+		Do("READONLY", "maybe").Err("invalid argument 'maybe'"),
+	)
+}