diff --git a/CHANGELOG.md b/CHANGELOG.md index c7ea51f505..f2024a5323 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 9856e94aa9..935671db8b 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/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 diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index 1e23c3d290..8e571d35b8 100644 --- a/src/PostgREST/DbStructure.hs +++ b/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 diff --git a/src/PostgREST/OpenAPI.hs b/src/PostgREST/OpenAPI.hs index 6fcf943979..2719c2b3af 100644 --- a/src/PostgREST/OpenAPI.hs +++ b/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 diff --git a/src/PostgREST/QueryBuilder.hs b/src/PostgREST/QueryBuilder.hs index 635e4b3456..670f3c36f8 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/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 diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index 0a74bffba5..ea934b5b73 100644 --- a/src/PostgREST/Types.hs +++ b/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 diff --git a/test/Feature/RpcSpec.hs b/test/Feature/RpcSpec.hs index ede880728c..4036e1058a 100644 --- a/test/Feature/RpcSpec.hs +++ b/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] } diff --git a/test/Feature/StructureSpec.hs b/test/Feature/StructureSpec.hs index 02ce083be0..c288edb8f7 100644 --- a/test/Feature/StructureSpec.hs +++ b/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" diff --git a/test/QueryCost.hs b/test/QueryCost.hs index 56953280c1..074b17d16f 100644 --- a/test/QueryCost.hs +++ b/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,14 +41,14 @@ 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) @@ -56,7 +56,7 @@ main = do 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) diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 1e36b589c9..f599aa1d6d 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -221,28 +221,56 @@ CREATE FUNCTION varied_arguments( date date, money money, enum enum_menagerie_type, + arr text[], "integer" integer default 42, json json default '{}', jsonb jsonb default '{}' -) RETURNS text - LANGUAGE sql +) RETURNS json +LANGUAGE sql AS $_$ - SELECT 'Hi'::text; + SELECT json_build_object( + 'double', double, + 'varchar', "varchar", + 'boolean', "boolean", + 'date', date, + 'money', money, + 'enum', enum, + 'arr', arr, + 'integer', "integer", + 'json', json, + 'jsonb', jsonb + ); $_$; -COMMENT ON FUNCTION varied_arguments(double precision, character varying, boolean, date, money, enum_menagerie_type, integer, json, jsonb) IS +COMMENT ON FUNCTION varied_arguments(double precision, character varying, boolean, date, money, enum_menagerie_type, text[], integer, json, jsonb) IS $_$An RPC function Just a test for RPC function arguments$_$; CREATE FUNCTION json_argument(arg json) RETURNS text - LANGUAGE sql AS $_$ SELECT json_typeof(arg); $_$; + +CREATE FUNCTION array_argument(arg TEXT[]) RETURNS text[] +LANGUAGE SQL AS $_$ + SELECT arg +$_$; + + +CREATE FUNCTION generic_argument(arg INT[]) RETURNS INT[] +LANGUAGE SQL AS $_$ + SELECT arg +$_$; + +CREATE FUNCTION generic_argument(arg INT) RETURNS INT[] +LANGUAGE SQL AS $_$ + SELECT generic_argument(ARRAY[arg]) +$_$; + -- -- Name: jwt_test(); Type: FUNCTION; Schema: test; Owner: - -- @@ -1072,6 +1100,11 @@ create function test.many_inout_params(INOUT num int, INOUT str text, INOUT b bo select num, str, b; $$ language sql; +CREATE FUNCTION test.variadic_param(VARIADIC v TEXT[]) RETURNS text[] +LANGUAGE SQL AS $_$ + SELECT v +$_$; + create or replace function test.raise_pt402() returns void as $$ begin raise sqlstate 'PT402' using message = 'Payment Required',