diff --git a/CHANGELOG.md b/CHANGELOG.md index 119f15c2a..5d0aea43f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #1512, Allow schema cache reloading with NOTIFY - @steve-chavez - #1119, Allow config file reloading with SIGUSR2 - @steve-chavez - #1558, Allow 'Bearer' with and without capitalization as authentication schema - @wolfgangwalther + - #1470, Allow calling RPC with variadic argument by passing repeated params - @wolfgangwalther - #1559, No downtime when reloading the schema cache with SIGUSR1 - @steve-chavez - #504, Add `log-level` config option. The admitted levels are: crit, error, warn and info - @steve-chavez diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 2dffadad2..386b2c666 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -179,10 +179,14 @@ userApiRequest confSchemas rootSpec dbStructure req reqBody | otherwise = Nothing parsedColumns = pRequestColumns columns payloadColumns = - case (relevantPayload, fromRight Nothing parsedColumns) of - (Just ProcessedJSON{pjKeys}, _) -> pjKeys - (Just RawJSON{}, Just cls) -> cls - _ -> S.empty + case (contentType, action) of + (_, ActionInvoke InvGet) -> S.fromList $ fst <$> rpcQParams + (_, ActionInvoke InvHead) -> S.fromList $ fst <$> rpcQParams + (CTOther "application/x-www-form-urlencoded", _) -> S.fromList $ map (toS . fst) $ parseSimpleQuery $ toS reqBody + _ -> case (relevantPayload, fromRight Nothing parsedColumns) of + (Just ProcessedJSON{pjKeys}, _) -> pjKeys + (Just RawJSON{}, Just cls) -> cls + _ -> S.empty payload = case (contentType, action) of (_, ActionInvoke InvGet) -> Right rpcPrmsToJson @@ -198,12 +202,18 @@ userApiRequest confSchemas rootSpec dbStructure 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 = paramsFromList . map (toS *** 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 $ paramsFromList rpcQParams) (S.fromList $ fst <$> rpcQParams) + paramsFromList ls = M.fromListWith mergeParams $ toRpcParamsWith isVariadic ls + where + isVariadic k = + case target of + TargetProc{tProc} -> argIsVariadic tProc k + _ -> False 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 1e23c3d29..8e571d35b 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 6fcf94397..2719c2b3a 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 30b369028..006ea5d6a 100644 --- a/src/PostgREST/QueryBuilder.hs +++ b/src/PostgREST/QueryBuilder.hs @@ -137,15 +137,18 @@ requestToCallProcQuery qi pgArgs returnsScalar preferParams returnings = BS.unwords [ normalizedBody <> ",", "pgrst_args AS (", - "SELECT * FROM json_to_recordset(" <> selectBody <> ") AS _(" <> fmtArgs (\a -> " " <> encodeUtf8 (pgaType a)) <> ")", + "SELECT * FROM json_to_recordset(" <> selectBody <> ") AS _(" <> fmtArgs (const mempty) (\a -> " " <> encodeUtf8 (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 = BS.intercalate ", " ((\a -> pgFmtIdent (pgaName a) <> argFrag a) <$> pgArgs) + fmtArgs :: (PgArg -> SqlFragment) -> (PgArg -> SqlFragment) -> SqlFragment + fmtArgs argFragPre argFragSuf = BS.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 034f52e76..a3ee004ea 100644 --- a/src/PostgREST/Types.hs +++ b/src/PostgREST/Types.hs @@ -4,6 +4,7 @@ Description : PostgREST common types and functions used by the rest of the modul -} {-# LANGUAGE DeriveGeneric #-} {-# LANGUAGE DuplicateRecordFields #-} +{-# LANGUAGE NamedFieldPuns #-} module PostgREST.Types where @@ -123,6 +124,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) @@ -180,7 +182,7 @@ findProc qi payloadKeys paramsAsSingleObject allProcs = fromMaybe fallback bestM -} specifiedProcArgs :: S.Set FieldName -> ProcDescription -> [PgArg] specifiedProcArgs keys proc = - (\k -> fromMaybe (PgArg k "text" True) (find ((==) k . pgaName) (pdArgs proc))) <$> S.toList keys + (\k -> fromMaybe (PgArg k "text" True False) (find ((==) k . pgaName) (pdArgs proc))) <$> S.toList keys procReturnsScalar :: ProcDescription -> Bool procReturnsScalar proc = case proc of @@ -193,6 +195,12 @@ procTableName proc = case pdReturnType proc of Single (Composite qi) -> Just $ qiName qi _ -> Nothing +argIsVariadic :: ProcDescription -> Text -> Bool +argIsVariadic proc arg = + case find (\PgArg{pgaName} -> pgaName == arg) $ pdArgs proc of + Just PgArg{pgaVar} -> pgaVar + _ -> False + type Schema = Text type TableName = Text @@ -399,8 +407,26 @@ type Alias = Text type Cast = Text type NodeName = Text --- Rpc query param, only used for GET rpcs -type RpcQParam = (Text, Text) +-- RPC query param, used for POST of form-data and GET requests +data RpcParamValue = Fixed Text | Variadic [Text] + +mergeParams :: RpcParamValue -> RpcParamValue -> RpcParamValue +mergeParams (Variadic a) (Variadic b) = Variadic $ b ++ a +-- repeated params for non-variadic arguments are not merged +mergeParams _ v = v + +instance JSON.ToJSON RpcParamValue where + toJSON (Fixed v) = JSON.toJSON v + toJSON (Variadic v) = JSON.toJSON v + +type RpcParams = [(Text, RpcParamValue)] + +toRpcParamsWith :: (Text -> Bool) -> [(Text, Text)] -> RpcParams +toRpcParamsWith isVariadic ls = toRpcParamValue <$> ls + where + toRpcParamValue (k, v) + | isVariadic k = (k, Variadic [v]) + | otherwise = (k, Fixed v) {-| Custom guc header, it's obtained by parsing the json in a: diff --git a/test/Feature/RpcSpec.hs b/test/Feature/RpcSpec.hs index 1afc2016f..2251d7813 100644 --- a/test/Feature/RpcSpec.hs +++ b/test/Feature/RpcSpec.hs @@ -469,6 +469,64 @@ spec actualPgVersion = get "/rpc/many_inout_params?num=1&str=two&b=false" `shouldRespondWith` [json| [{"num":1,"str":"two","b":false}]|] { matchHeaders = [matchContentTypeJson] } + context "procs with VARIADIC params" $ do + when (actualPgVersion < pgVersion100) $ + it "works with POST (Postgres < 10)" $ + post "/rpc/variadic_param" + [json| { "v": "{hi,hello,there}" } |] + `shouldRespondWith` + [json|["hi", "hello", "there"]|] + + when (actualPgVersion >= pgVersion100) $ do + it "works with POST (Postgres >= 10)" $ + post "/rpc/variadic_param" + [json| { "v": ["hi", "hello", "there"] } |] + `shouldRespondWith` + [json|["hi", "hello", "there"]|] + + context "works with GET and repeated params" $ do + it "n=0 (through DEFAULT)" $ + get "/rpc/variadic_param" + `shouldRespondWith` + [json|[]|] + + it "n=1" $ + get "/rpc/variadic_param?v=hi" + `shouldRespondWith` + [json|["hi"]|] + + it "n>1" $ + get "/rpc/variadic_param?v=hi&v=there" + `shouldRespondWith` + [json|["hi", "there"]|] + + context "works with POST and repeated params from html form" $ do + it "n=0 (through DEFAULT)" $ + request methodPost "/rpc/variadic_param" + [("Content-Type", "application/x-www-form-urlencoded")] + "" + `shouldRespondWith` + [json|[]|] + + it "n=1" $ + request methodPost "/rpc/variadic_param" + [("Content-Type", "application/x-www-form-urlencoded")] + "v=hi" + `shouldRespondWith` + [json|["hi"]|] + + it "n>1" $ + request methodPost "/rpc/variadic_param" + [("Content-Type", "application/x-www-form-urlencoded")] + "v=hi&v=there" + `shouldRespondWith` + [json|["hi", "there"]|] + + it "returns first value for repeated params without VARIADIC" $ + get "/rpc/sayhello?name=world&name=ignored" + `shouldRespondWith` + [json|"Hello, world"|] + 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/QueryCost.hs b/test/QueryCost.hs index 5c72e15de..cffaeeec6 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 c56587059..ea1cd48fb 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -1083,6 +1083,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[] DEFAULT '{}') 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',