fix calling rpc with variadic argument

CHANGELOG.md

@@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 ### Fixed
 
  - #1530, Fix how the PostgREST version is shown in the help text when the `.git` directory is not available - @monacoremo
+ - #1552, Fix calling RPC with variadic argument by passing an array (#1470) - @wolfgangwalther
  - #1094, Fix expired JWTs starting an empty transaction on the db - @steve-chavez
  - #1475, Fix location header for POST request with select= without PK (#1162) - @wolfgangwalther
 

src/PostgREST/ApiRequest.hs

@@ -189,12 +189,19 @@ userApiRequest confSchemas rootSpec req reqBody
         json <- csvToJson <$> CSV.decodeByName reqBody
         note "All lines must have same number of fields" $ payloadAttributes (JSON.encode json) json
       (CTOther "application/x-www-form-urlencoded", _) ->
-        let json = M.fromList . map (toS *** JSON.String . toS) . parseSimpleQuery $ toS reqBody
+        let json = mapFromListWithMultiple . map (toS *** JSON.String . toS) . parseSimpleQuery $ toS reqBody
             keys = S.fromList $ M.keys json in
         Right $ ProcessedJSON (JSON.encode json) keys
       (ct, _) ->
         Left $ toS $ "Content-Type not acceptable: " <> toMime ct
-  rpcPrmsToJson = ProcessedJSON (JSON.encode $ M.fromList $ second JSON.toJSON <$> rpcQParams) (S.fromList $ fst <$> rpcQParams)
+  rpcPrmsToJson = ProcessedJSON (JSON.encode $ mapFromListWithMultiple $ second JSON.toJSON <$> rpcQParams) (S.fromList $ fst <$> rpcQParams)
+  mapFromListWithMultiple ls = map listToValue $ M.fromListWith reverseConcat . map valueToList $ ls
+    where
+        reverseConcat a b = b ++ a
+        valueToList (k,v) = (k,[v])
+        listToValue xs = case xs of
+          [v] -> v
+          _   -> JSON.toJSON xs
   topLevelRange = fromMaybe allRange $ M.lookup "limit" ranges -- if no limit is specified, get all the request rows
   action =
     case method of

src/PostgREST/DbStructure.hs

@@ -149,14 +149,16 @@ decodeProcs =
     parseArgs = mapMaybe parseArg . filter (not . isPrefixOf "OUT" . toS) . map strip . split (==',')
 
     parseArg :: Text -> Maybe PgArg
-    parseArg a =
-      let arg = lastDef "" $ splitOn "INOUT " a
-          (body, def) = breakOn " DEFAULT " arg
+    parseArg arg =
+      let isVariadic = isPrefixOf "VARIADIC " $ toS arg
+          -- argmode can be IN, OUT, INOUT, or VARIADIC
+          argNoMode = lastDef "" $ splitOn (if isVariadic then "VARIADIC " else "INOUT ") arg
+          (body, def) = breakOn " DEFAULT " argNoMode
           (name, typ) = breakOn " " body in
       if T.null typ
          then Nothing
          else Just $
-           PgArg (dropAround (== '"') name) (strip typ) (T.null def)
+           PgArg (dropAround (== '"') name) (strip typ) (T.null def) isVariadic
 
     parseRetType :: Text -> Text -> Bool -> Char -> RetType
     parseRetType schema name isSetOf typ

src/PostgREST/OpenAPI.hs

@@ -96,7 +96,7 @@ makeProcSchema pd =
   & required .~ map pgaName (filter pgaReq (pdArgs pd))
 
 makeProcProperty :: PgArg -> (Text, Referenced Schema)
-makeProcProperty (PgArg n t _) = (n, Inline s)
+makeProcProperty (PgArg n t _ _) = (n, Inline s)
   where
     s = (mempty :: Schema)
           & type_ ?~ toSwaggerType t

src/PostgREST/QueryBuilder.hs

@@ -137,15 +137,18 @@ requestToCallProcQuery qi pgArgs returnsScalar preferParams returnings =
           unwords [
             normalizedBody <> ",",
             "pgrst_args AS (",
-              "SELECT * FROM json_to_recordset(" <> selectBody <> ") AS _(" <> fmtArgs (\a -> " " <> pgaType a) <> ")",
+              "SELECT * FROM json_to_recordset(" <> selectBody <> ") AS _(" <> fmtArgs (const mempty) (\a -> " " <> pgaType a) <> ")",
             ")"]
          , if paramsAsMultipleObjects
-             then fmtArgs (\a -> " := pgrst_args." <> pgFmtIdent (pgaName a))
-             else fmtArgs (\a -> " := (SELECT " <> pgFmtIdent (pgaName a) <> " FROM pgrst_args LIMIT 1)")
+             then fmtArgs varadicPrefix (\a -> " := pgrst_args." <> pgFmtIdent (pgaName a))
+             else fmtArgs varadicPrefix (\a -> " := (SELECT " <> pgFmtIdent (pgaName a) <> " FROM pgrst_args LIMIT 1)")
         )
 
-    fmtArgs :: (PgArg -> SqlFragment) -> SqlFragment
-    fmtArgs argFrag = intercalate ", " ((\a -> pgFmtIdent (pgaName a) <> argFrag a) <$> pgArgs)
+    fmtArgs :: (PgArg -> SqlFragment) -> (PgArg -> SqlFragment) -> SqlFragment
+    fmtArgs argFragPre argFragSuf = intercalate ", " ((\a -> argFragPre a <> pgFmtIdent (pgaName a) <> argFragSuf a) <$> pgArgs)
+
+    varadicPrefix :: PgArg -> SqlFragment
+    varadicPrefix a = if pgaVar a then "VARIADIC " else mempty
 
     sourceBody :: SqlFragment
     sourceBody

src/PostgREST/Types.hs

@@ -123,6 +123,7 @@ data PgArg = PgArg {
   pgaName :: Text
 , pgaType :: Text
 , pgaReq  :: Bool
+, pgaVar  :: Bool
 } deriving (Show, Eq, Ord)
 
 data PgType = Scalar QualifiedIdentifier | Composite QualifiedIdentifier deriving (Eq, Show, Ord)
@@ -179,7 +180,7 @@ specifiedProcArgs keys proc =
   let
     args = maybe [] pdArgs proc
   in
-  (\k -> fromMaybe (PgArg k "text" True) (find ((==) k . pgaName) args)) <$> S.toList keys
+  (\k -> fromMaybe (PgArg k "text" True False) (find ((==) k . pgaName) args)) <$> S.toList keys
 
 procReturnsScalar :: ProcDescription -> Bool
 procReturnsScalar proc = case proc of

test/Feature/RpcSpec.hs

@@ -274,21 +274,102 @@ spec actualPgVersion =
           { matchHeaders = [matchContentTypeJson] }
 
     context "proc argument types" $ do
-      it "accepts a variety of arguments" $
-        post "/rpc/varied_arguments"
-            [json| {
-              "double": 3.1,
-              "varchar": "hello",
-              "boolean": true,
-              "date": "20190101",
-              "money": 0,
-              "enum": "foo",
-              "integer": 43,
-              "json": {"some key": "some value"},
-              "jsonb": {"another key": [1, 2, "3"]}
-            } |]
+      -- different syntax for array needed for pg<10
+      when (actualPgVersion < pgVersion100) $
+        it "accepts a variety of arguments (Postgres < 10)" $
+          post "/rpc/varied_arguments"
+              [json| {
+                "double": 3.1,
+                "varchar": "hello",
+                "boolean": true,
+                "date": "20190101",
+                "money": 0,
+                "enum": "foo",
+                "arr": "{a,b,c}",
+                "integer": 43,
+                "json": {"some key": "some value"},
+                "jsonb": {"another key": [1, 2, "3"]}
+              } |]
+            `shouldRespondWith`
+              [json| {
+                "double": 3.1,
+                "varchar": "hello",
+                "boolean": true,
+                "date": "2019-01-01",
+                "money": "$0.00",
+                "enum": "foo",
+                "arr": ["a", "b", "c"],
+                "integer": 43,
+                "json": {"some key": "some value"},
+                "jsonb": {"another key": [1, 2, "3"]}
+              } |]
+              { matchHeaders = [matchContentTypeJson] }
+
+      when (actualPgVersion >= pgVersion100) $
+        it "accepts a variety of arguments (Postgres >= 10)" $
+          post "/rpc/varied_arguments"
+              [json| {
+                "double": 3.1,
+                "varchar": "hello",
+                "boolean": true,
+                "date": "20190101",
+                "money": 0,
+                "enum": "foo",
+                "arr": ["a", "b", "c"],
+                "integer": 43,
+                "json": {"some key": "some value"},
+                "jsonb": {"another key": [1, 2, "3"]}
+              } |]
+            `shouldRespondWith`
+              [json| {
+                "double": 3.1,
+                "varchar": "hello",
+                "boolean": true,
+                "date": "2019-01-01",
+                "money": "$0.00",
+                "enum": "foo",
+                "arr": ["a", "b", "c"],
+                "integer": 43,
+                "json": {"some key": "some value"},
+                "jsonb": {"another key": [1, 2, "3"]}
+              } |]
+              { matchHeaders = [matchContentTypeJson] }
+
+      it "accepts a variety of arguments with GET" $
+        -- without JSON / JSONB here, because passing those via query string is useless - they just become a "json string" all the time
+        get "/rpc/varied_arguments?double=3.1&varchar=hello&boolean=true&date=20190101&money=0&enum=foo&arr=%7Ba,b,c%7D&integer=43"
+          `shouldRespondWith`
+              [json| {
+                "double": 3.1,
+                "varchar": "hello",
+                "boolean": true,
+                "date": "2019-01-01",
+                "money": "$0.00",
+                "enum": "foo",
+                "arr": ["a", "b", "c"],
+                "integer": 43,
+                "json": {},
+                "jsonb": {}
+              } |]
+            { matchHeaders = [matchContentTypeJson] }
+
+      it "accepts a variety of arguments from an html form" $
+        request methodPost "/rpc/varied_arguments"
+            [("Content-Type", "application/x-www-form-urlencoded")]
+            "double=3.1&varchar=hello&boolean=true&date=20190101&money=0&enum=foo&arr=%7Ba,b,c%7D&integer=43"
           `shouldRespondWith`
-            [json|"Hi"|]
+              [json| {
+                "double": 3.1,
+                "varchar": "hello",
+                "boolean": true,
+                "date": "2019-01-01",
+                "money": "$0.00",
+                "enum": "foo",
+                "arr": ["a", "b", "c"],
+                "integer": 43,
+                "json": {},
+                "jsonb": {}
+              } |]
             { matchHeaders = [matchContentTypeJson] }
 
       it "parses embedded JSON arguments as JSON" $
@@ -315,6 +396,47 @@ spec actualPgVersion =
               [json|"string"|]
               { matchHeaders = [matchContentTypeJson] }
 
+    context "repeated params as array" $ do
+      context "in query string for GET" $ do
+        it "without single item overload" $ do
+          get "/rpc/array_argument?arg=a&arg=b&arg=c"
+            `shouldRespondWith`
+              [json|["a", "b", "c"]|]
+          get "/rpc/array_argument?arg=a"
+            `shouldRespondWith` 400
+
+        it "with single item overload" $ do
+          get "/rpc/generic_argument?arg=1&arg=2&arg=3"
+            `shouldRespondWith`
+              [json|[1, 2, 3]|]
+          get "/rpc/generic_argument?arg=1"
+            `shouldRespondWith`
+              [json|[1]|]
+
+      context "in x-www-form-urlencoded body for POST" $ do
+        it "without single item overload" $ do
+          request methodPost "/rpc/array_argument"
+              [("Content-Type", "application/x-www-form-urlencoded")]
+              "arg=a&arg=b&arg=c"
+            `shouldRespondWith`
+              [json|["a", "b", "c"]|]
+          request methodPost "/rpc/array_argument"
+              [("Content-Type", "application/x-www-form-urlencoded")]
+              "arg=a"
+            `shouldRespondWith` 400
+
+        it "with single item overload" $ do
+          request methodPost "/rpc/generic_argument"
+              [("Content-Type", "application/x-www-form-urlencoded")]
+              "arg=1&arg=2&arg=3"
+            `shouldRespondWith`
+              [json|[1, 2, 3]|]
+          request methodPost "/rpc/generic_argument"
+              [("Content-Type", "application/x-www-form-urlencoded")]
+              "arg=1"
+            `shouldRespondWith`
+              [json|[1]|]
+
     context "improper input" $ do
       it "rejects unknown content type even if payload is good" $ do
         request methodPost "/rpc/sayhello"
@@ -394,6 +516,41 @@ spec actualPgVersion =
         get "/rpc/many_inout_params?num=1&str=two&b=false" `shouldRespondWith`
           [json| [{"num":1,"str":"two","b":false}]|] { matchHeaders = [matchContentTypeJson] }
 
+    context "with variadic parameter" $ do
+      when (actualPgVersion < pgVersion100) $
+        it "works with POST (Postgres < 10)" $
+          post "/rpc/variadic_param"
+              [json| { "v": "{hi,hello,there}" } |]
+            `shouldRespondWith`
+              [json|["hi", "hello", "there"]|]
+              { matchHeaders = [matchContentTypeJson] }
+
+      when (actualPgVersion >= pgVersion100) $
+        it "works with POST (Postgres >= 10)" $
+          post "/rpc/variadic_param"
+              [json| { "v": ["hi", "hello", "there"] } |]
+            `shouldRespondWith`
+              [json|["hi", "hello", "there"]|]
+              { matchHeaders = [matchContentTypeJson] }
+
+      it "works with GET" $
+        get "/rpc/variadic_param?v=%7Bhi,hello,there%7D"
+          `shouldRespondWith`
+            [json|["hi", "hello", "there"]|]
+            { matchHeaders = [matchContentTypeJson] }
+
+      it "works with repeated params" $
+        get "/rpc/variadic_param?v=hi&v=hello&v=there"
+          `shouldRespondWith`
+            [json|["hi", "hello", "there"]|]
+            { matchHeaders = [matchContentTypeJson] }
+
+      it "works with single param" $
+        get "/rpc/variadic_param?v=hi"
+          `shouldRespondWith`
+            [json|["hi"]|]
+            { matchHeaders = [matchContentTypeJson] }
+
     it "can handle procs with args that have a DEFAULT value" $ do
       get "/rpc/many_inout_params?num=1&str=two" `shouldRespondWith`
         [json| [{"num":1,"str":"two","b":true}]|] { matchHeaders = [matchContentTypeJson] }

test/Feature/StructureSpec.hs

@@ -425,7 +425,8 @@ spec = do
                   "boolean",
                   "date",
                   "money",
-                  "enum"
+                  "enum",
+                  "arr"
                 ],
                 "properties": {
                   "double": {
@@ -452,6 +453,10 @@ spec = do
                     "format": "enum_menagerie_type",
                     "type": "string"
                   },
+                  "arr": {
+                    "format": "text[]",
+                    "type": "string"
+                  },
                   "integer": {
                     "format": "integer",
                     "type": "integer"

test/QueryCost.hs

@@ -29,7 +29,7 @@ main = do
     context "call proc query" $ do
       it "should not exceed cost when calling setof composite proc" $ do
         cost <- exec pool [str| {"id": 3} |] $
-          requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [PgArg "id" "int" True] False Nothing []
+          requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [PgArg "id" "int" True False] False Nothing []
         liftIO $
           cost `shouldSatisfy` (< Just 40)
 
@@ -41,22 +41,22 @@ main = do
 
       it "should not exceed cost when calling scalar proc" $ do
         cost <- exec pool [str| {"a": 3, "b": 4} |] $
-          requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [PgArg "a" "int" True, PgArg "b" "int" True] True Nothing []
+          requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [PgArg "a" "int" True False, PgArg "b" "int" True False] True Nothing []
         liftIO $
           cost `shouldSatisfy` (< Just 10)
 
       context "params=multiple-objects" $ do
         it "should not exceed cost when calling setof composite proc" $ do
           cost <- exec pool [str| [{"id": 1}, {"id": 4}] |] $
-            requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [PgArg "id" "int" True] False (Just MultipleObjects) []
+            requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [PgArg "id" "int" True False] False (Just MultipleObjects) []
           liftIO $ do
             -- lower bound needed for now to make sure that cost is not Nothing
             cost `shouldSatisfy` (> Just 2000)
             cost `shouldSatisfy` (< Just 2100)
 
         it "should not exceed cost when calling scalar proc" $ do
           cost <- exec pool [str| [{"a": 3, "b": 4}, {"a": 1, "b": 2}, {"a": 8, "b": 7}] |] $
-            requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [PgArg "a" "int" True, PgArg "b" "int" True] True Nothing []
+            requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [PgArg "a" "int" True False, PgArg "b" "int" True False] True Nothing []
           liftIO $
             cost `shouldSatisfy` (< Just 10)