Merge pull request #112 from NoRedInk/phx-1626-fix-eval-argument-orde…

…ring Fix ordering of `script` argument interpolation
NoRedInk · Jun 3, 2024 · 733ee55 · 733ee55
2 parents cb753bc + a55a143
commit 733ee55
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 18 deletions.
diff --git a/nri-redis/src/Redis/Script.hs b/nri-redis/src/Redis/Script.hs
@@ -15,7 +15,6 @@ module Redis.Script
     Tokens (..),
     ScriptParam (..),
     HasScriptParam (..),
-    printScript,
   )
 where
 
@@ -201,8 +200,8 @@ scriptFromEvaluatedTokens quasiQuotedString' evaluatedTokens =
    in Script
         { luaScript = buffer script',
           quasiQuotedString = quasiQuotedString',
-          keys = keyList script',
-          arguments = Log.mkSecret (argList script')
+          keys = List.reverse (keyList script'),
+          arguments = Log.mkSecret (List.reverse (argList script'))
         }
 
 -----------------------------
@@ -275,12 +274,3 @@ toHex bytes =
     |> List.map (Text.Printf.printf "%02x")
     |> List.concat
     |> Text.fromList
-
----------------------------------------------
--- Helper functions for testing
----------------------------------------------
-
-printScript :: Script a -> Text
-printScript Script {luaScript, quasiQuotedString, keys, arguments} =
-  let listStr l = List.map (\s -> "\"" ++ s ++ "\"") l |> Text.join ", "
-   in "Script { luaScript = \"" ++ luaScript ++ "\", quasiQuotedString = \"" ++ quasiQuotedString ++ "\", keys = [" ++ listStr keys ++ "], arguments = [" ++ listStr (Log.unSecret arguments) ++ "] }"
diff --git a/nri-redis/test/Spec/Redis.hs b/nri-redis/test/Spec/Redis.hs
@@ -400,9 +400,11 @@ queryTests redisHandler =
             [Redis.script|
       local a = ${Redis.Key 2}
       local b = ${Redis.Literal 3}
-      return b|]
+      local c = ${Redis.Literal 4}
+      local d = ${Redis.Literal 5}
+      return {b, c, d}|]
       result <- Redis.eval testNS script |> Expect.succeeds
-      Expect.equal result 3,
+      Expect.equal result [3, 4, 5],
     Test.test "eval with arguments namespaces key" <| \() -> do
       let script = [Redis.script|return ${Redis.Key "hi"}|]
       (result :: Text) <- Redis.eval testNS script |> Expect.succeeds

diff --git a/nri-redis/test/Spec/Redis/Script.hs b/nri-redis/test/Spec/Redis/Script.hs
@@ -113,12 +113,54 @@ thTests :: List Test.Test
 thTests =
   [ Test.test "just text" <| \_ ->
       [script|some text|]
-        |> printScript
-        |> Expect.equal "Script { luaScript = \"some text\", quasiQuotedString = \"some text\", keys = [], arguments = [] }",
+        |> Expect.equal
+          ( Script
+              { luaScript = "some text",
+                quasiQuotedString = "some text",
+                keys = [],
+                arguments = Log.mkSecret []
+              }
+          ),
     Test.test "one key argument" <| \_ ->
       [script|${Key "hi"}|]
-        |> printScript
-        |> Expect.equal "Script { luaScript = \"KEYS[1]\", quasiQuotedString = \"${Key \"hi\"}\", keys = [\"hi\"], arguments = [] }",
+        |> Expect.equal
+          ( Script
+              { luaScript = "KEYS[1]",
+                quasiQuotedString = "${Key \"hi\"}",
+                keys = ["hi"],
+                arguments = Log.mkSecret []
+              }
+          ),
+    Test.test "one literal argument" <| \_ ->
+      [script|${Literal "hi"}|]
+        |> Expect.equal
+          ( Script
+              { luaScript = "ARGV[1]",
+                quasiQuotedString = "${Literal \"hi\"}",
+                keys = [],
+                arguments = Log.mkSecret ["hi"]
+              }
+          ),
+    Test.test "one key one literal argument" <| \_ ->
+      [script|${Key "a key"} ${Literal "a literal"}|]
+        |> Expect.equal
+          ( Script
+              { luaScript = "KEYS[1] ARGV[1]",
+                quasiQuotedString = "${Key \"a key\"} ${Literal \"a literal\"}",
+                keys = ["a key"],
+                arguments = Log.mkSecret ["a literal"]
+              }
+          ),
+    Test.test "multiple keys and literals" <| \_ ->
+      [script|${Key "key1"} ${Key "key2"} ${Key "key3"} ${Literal "literal1"} ${Literal "literal2"} ${Literal "literal3"}|]
+        |> Expect.equal
+          ( Script
+              { luaScript = "KEYS[1] KEYS[2] KEYS[3] ARGV[1] ARGV[2] ARGV[3]",
+                quasiQuotedString = "${Key \"key1\"} ${Key \"key2\"} ${Key \"key3\"} ${Literal \"literal1\"} ${Literal \"literal2\"} ${Literal \"literal3\"}",
+                keys = ["key1", "key2", "key3"],
+                arguments = Log.mkSecret ["literal1", "literal2", "literal3"]
+              }
+          ),
     Test.test "fails on type-checking when not given Key or Literal" <| \_ ->
       [script|${False}|]
         |> arguments