jimmy/postgrest
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

refactor: PgArg to ProcParam

Browse Source 
Clarify the difference between arguments and parameters.
Parameters are part of the function definition, arguments are the values
passed to the function.

Also clarify the findProc function comments and error message.
This commit is contained in:
steve-chavez
2021-08-30 18:17:59 -05:00
committed by Steve Chavez
parent ed5072f4b1
commit c9a60373f6
9 changed files with 96 additions and 97 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/PostgREST/App.hs
+1 -1
View File
@@ -442,7 +442,7 @@ handleInvoke invMethod proc context@RequestContext{..} = do
(returnsSingle iTarget)
(QueryBuilder.requestToCallProcQuery
(QualifiedIdentifier (pdSchema proc) (pdName proc))
(Proc.specifiedProcArgs iColumns proc)
(Proc.specifiedProcParams iColumns proc)
iPayload
(returnsScalar iTarget)
iPreferParameters
src/PostgREST/DbStructure.hs
+3 -2
View File
@@ -43,8 +43,9 @@ import Text.InterpolatedString.Perl6 (q)
import PostgREST.DbStructure.Identifiers (QualifiedIdentifier (..),
Schema, TableName)
import PostgREST.DbStructure.Proc (PgArg (..), PgType (..),
import PostgREST.DbStructure.Proc (PgType (..),
ProcDescription (..),
ProcParam (..),
ProcVolatility (..),
ProcsMap, RetType (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
@@ -192,7 +193,7 @@ decodeProcs =
<*> column HD.text
<*> nullableColumn HD.text
<*> compositeArrayColumn
(PgArg
(ProcParam
<$> compositeField HD.text
<*> compositeField HD.text
<*> compositeField HD.bool
src/PostgREST/DbStructure/Proc.hs
+20 -21
View File
@@ -2,16 +2,16 @@
{-# LANGUAGE DeriveGeneric #-}
module PostgREST.DbStructure.Proc
( PgArg(..)
, PgType(..)
( PgType(..)
, ProcDescription(..)
, ProcParam(..)
, ProcVolatility(..)
, ProcsMap
, RetType(..)
, procReturnsScalar
, procReturnsSingle
, procTableName
, specifiedProcArgs
, specifiedProcParams
) where
import qualified Data.Aeson as JSON
@@ -24,15 +24,6 @@ import PostgREST.DbStructure.Identifiers (FieldName,
import Protolude
data PgArg = PgArg
{ pgaName :: Text
, pgaType :: Text
, pgaReq :: Bool
, pgaVar :: Bool
}
deriving (Eq, Ord, Generic, JSON.ToJSON)
data PgType
= Scalar
| Composite QualifiedIdentifier
@@ -53,19 +44,27 @@ data ProcDescription = ProcDescription
{ pdSchema :: Schema
, pdName :: Text
, pdDescription :: Maybe Text
, pdArgs :: [PgArg]
, pdParams :: [ProcParam]
, pdReturnType :: RetType
, pdVolatility :: ProcVolatility
, pdHasVariadic :: Bool
}
deriving (Eq, Generic, JSON.ToJSON)
-- Order by least number of args in the case of overloaded functions
data ProcParam = ProcParam
{ ppName :: Text
, ppType :: Text
, ppReq :: Bool
, ppVar :: Bool
}
deriving (Eq, Ord, Generic, JSON.ToJSON)
-- Order by least number of params in the case of overloaded functions
instance Ord ProcDescription where
ProcDescription schema1 name1 des1 args1 rt1 vol1 hasVar1 `compare` ProcDescription schema2 name2 des2 args2 rt2 vol2 hasVar2
| schema1 == schema2 && name1 == name2 && length args1 < length args2 = LT
| schema2 == schema2 && name1 == name2 && length args1 > length args2 = GT
| otherwise = (schema1, name1, des1, args1, rt1, vol1, hasVar1) `compare` (schema2, name2, des2, args2, rt2, vol2, hasVar2)
ProcDescription schema1 name1 des1 prms1 rt1 vol1 hasVar1 `compare` ProcDescription schema2 name2 des2 prms2 rt2 vol2 hasVar2
| schema1 == schema2 && name1 == name2 && length prms1 < length prms2 = LT
| schema2 == schema2 && name1 == name2 && length prms1 > length prms2 = GT
| otherwise = (schema1, name1, des1, prms1, rt1, vol1, hasVar1) `compare` (schema2, name2, des2, prms2, rt2, vol2, hasVar2)
-- | A map of all procs, all of which can be overloaded(one entry will have more than one ProcDescription).
-- | It uses a HashMap for a faster lookup.
@@ -75,9 +74,9 @@ type ProcsMap = M.HashMap QualifiedIdentifier [ProcDescription]
Search the procedure parameters by matching them with the specified keys.
If the key doesn't match a parameter, a parameter with a default type "text" is assumed.
-}
specifiedProcArgs :: S.Set FieldName -> ProcDescription -> [PgArg]
specifiedProcArgs keys proc =
(\k -> fromMaybe (PgArg k "text" True False) (find ((==) k . pgaName) (pdArgs proc))) <$> S.toList keys
specifiedProcParams :: S.Set FieldName -> ProcDescription -> [ProcParam]
specifiedProcParams keys proc =
(\k -> fromMaybe (ProcParam k "text" True False) (find ((==) k . ppName) (pdParams proc))) <$> S.toList keys
procReturnsScalar :: ProcDescription -> Bool
procReturnsScalar proc = case proc of
src/PostgREST/Error.hs
+10 -6
View File
@@ -29,8 +29,8 @@ import Network.HTTP.Types.Header (Header)
import PostgREST.ContentType (ContentType (..))
import qualified PostgREST.ContentType as ContentType
import PostgREST.DbStructure.Proc (PgArg (..),
ProcDescription (..))
import PostgREST.DbStructure.Proc (ProcDescription (..),
ProcParam (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
Junction (..),
Relationship (..))
@@ -99,11 +99,15 @@ instance JSON.ToJSON ApiRequestError where
"message" .= ("More than one relationship was found for " <> parent <> " and " <> child :: Text),
"details" .= (compressedRel <$> rels) ]
toJSON (AmbiguousRpc procs) = JSON.object [
"hint" .= ("Overloaded functions with the same argument name but different types are not supported" :: Text),
"message" .= ("Could not choose the best candidate function between: " <> T.intercalate ", " [pdSchema p <> "." <> pdName p <> "(" <> T.intercalate ", " [pgaName a <> " => " <> pgaType a | a <- pdArgs p] <> ")" | p <- procs])]
"hint" .= ("Overloaded functions with the same parameter name but different types are not supported" :: Text),
"message" .= ("Could not choose the best candidate function between: " <> T.intercalate ", " [pdSchema p <> "." <> pdName p <> "(" <> T.intercalate ", " [ppName a <> " => " <> ppType a | a <- pdParams p] <> ")" | p <- procs])]
toJSON (NoRpc schema procName payloadKeys hasPreferSingleObject) = JSON.object [
"hint" .= ("If a new function was created in the database with this name and arguments, try reloading the schema cache." :: Text),
"message" .= ("Could not find the " <> schema <> "." <> procName <> (if hasPreferSingleObject then " function with a single json or jsonb argument" else "(" <> T.intercalate ", " payloadKeys <> ")" <> " function") <> " in the schema cache")]
"hint" .= ("If a new function was created in the database with this name and parameters, try reloading the schema cache." :: Text),
"message" .= ("Could not find the " <> schema <> "." <> procName <>
(if hasPreferSingleObject
then " function with a single json or jsonb parameter"
else "(" <> T.intercalate ", " payloadKeys <> ")" <> " function") <>
" in the schema cache")]
toJSON UnsupportedVerb = JSON.object [
"message" .= ("Unsupported HTTP verb" :: Text)]
toJSON InvalidFilters = JSON.object [
src/PostgREST/OpenAPI.hs
+6 -6
View File
@@ -27,8 +27,8 @@ import PostgREST.Config (AppConfig (..), Proxy (..),
isMalformedProxyUri, toURI)
import PostgREST.DbStructure (DbStructure (..),
tableCols, tablePKCols)
import PostgREST.DbStructure.Proc (PgArg (..),
ProcDescription (..))
import PostgREST.DbStructure.Proc (ProcDescription (..),
ProcParam (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
PrimaryKey (..),
Relationship (..))
@@ -130,11 +130,11 @@ makeProcSchema pd =
(mempty :: Schema)
& description .~ pdDescription pd
& type_ ?~ SwaggerObject
& properties .~ fromList (fmap makeProcProperty (pdArgs pd))
& required .~ fmap pgaName (filter pgaReq (pdArgs pd))
& properties .~ fromList (fmap makeProcProperty (pdParams pd))
& required .~ fmap ppName (filter ppReq (pdParams pd))
makeProcProperty :: PgArg -> (Text, Referenced Schema)
makeProcProperty (PgArg n t _ _) = (n, Inline s)
makeProcProperty :: ProcParam -> (Text, Referenced Schema)
makeProcProperty (ProcParam n t _ _) = (n, Inline s)
where
s = (mempty :: Schema)
& type_ ?~ toSwaggerType t
src/PostgREST/Query/QueryBuilder.hs
+13 -13
View File
@@ -23,7 +23,7 @@ import Data.Tree (Tree (..))
import PostgREST.DbStructure.Identifiers (FieldName,
QualifiedIdentifier (..))
import PostgREST.DbStructure.Proc (PgArg (..))
import PostgREST.DbStructure.Proc (ProcParam (..))
import PostgREST.DbStructure.Relationship (Cardinality (..),
Relationship (..))
import PostgREST.DbStructure.Table (Table (..))
@@ -118,34 +118,34 @@ mutateRequestToQuery (Delete mainQi logicForest returnings) =
(if null logicForest then mempty else "WHERE " <> intercalateSnippet " AND " (map (pgFmtLogicTree mainQi) logicForest)) <> " " <>
H.sql (returningF mainQi returnings)
requestToCallProcQuery :: QualifiedIdentifier -> [PgArg] -> Maybe PayloadJSON -> Bool -> Maybe PreferParameters -> [FieldName] -> H.Snippet
requestToCallProcQuery qi pgArgs pj returnsScalar preferParams returnings =
argsCTE <> sourceBody
requestToCallProcQuery :: QualifiedIdentifier -> [ProcParam] -> Maybe PayloadJSON -> Bool -> Maybe PreferParameters -> [FieldName] -> H.Snippet
requestToCallProcQuery qi procParams pj returnsScalar preferParams returnings =
prmsCTE <> sourceBody
where
body = pjRaw <$> pj
paramsAsSingleObject = preferParams == Just SingleObject
paramsAsMultipleObjects = preferParams == Just MultipleObjects
(argsCTE, args)
| null pgArgs = (mempty, mempty)
(prmsCTE, args)
| null procParams = (mempty, mempty)
| paramsAsSingleObject = ("WITH pgrst_args AS (SELECT NULL)", jsonPlaceHolder body)
| otherwise = (
"WITH " <> normalizedBody body <> ", " <>
H.sql (
BS.unwords [
"pgrst_args AS (",
"SELECT * FROM json_to_recordset(" <> selectBody <> ") AS _(" <> fmtArgs (const mempty) (\a -> " " <> encodeUtf8 (pgaType a)) <> ")",
"SELECT * FROM json_to_recordset(" <> selectBody <> ") AS _(" <> fmtParams (const mempty) (\a -> " " <> encodeUtf8 (ppType a)) <> ")",
")"])
, H.sql $ if paramsAsMultipleObjects
then fmtArgs varadicPrefix (\a -> " := pgrst_args." <> pgFmtIdent (pgaName a))
else fmtArgs varadicPrefix (\a -> " := (SELECT " <> pgFmtIdent (pgaName a) <> " FROM pgrst_args LIMIT 1)")
then fmtParams varadicPrefix (\a -> " := pgrst_args." <> pgFmtIdent (ppName a))
else fmtParams varadicPrefix (\a -> " := (SELECT " <> pgFmtIdent (ppName a) <> " FROM pgrst_args LIMIT 1)")
)
fmtArgs :: (PgArg -> SqlFragment) -> (PgArg -> SqlFragment) -> SqlFragment
fmtArgs argFragPre argFragSuf = BS.intercalate ", " ((\a -> argFragPre a <> pgFmtIdent (pgaName a) <> argFragSuf a) <$> pgArgs)
fmtParams :: (ProcParam -> SqlFragment) -> (ProcParam -> SqlFragment) -> SqlFragment
fmtParams prmFragPre prmFragSuf = BS.intercalate ", " ((\a -> prmFragPre a <> pgFmtIdent (ppName a) <> prmFragSuf a) <$> procParams)
varadicPrefix :: PgArg -> SqlFragment
varadicPrefix a = if pgaVar a then "VARIADIC " else mempty
varadicPrefix :: ProcParam -> SqlFragment
varadicPrefix a = if ppVar a then "VARIADIC " else mempty
sourceBody :: H.Snippet
sourceBody
src/PostgREST/Request/ApiRequest.hs
+31 -36
View File
@@ -51,9 +51,8 @@ import PostgREST.DbStructure (DbStructure (..))
import PostgREST.DbStructure.Identifiers (FieldName,
QualifiedIdentifier (..),
Schema)
import PostgREST.DbStructure.Proc (PgArg (..),
ProcDescription (..),
ProcsMap)
import PostgREST.DbStructure.Proc (ProcDescription (..),
ProcParam (..), ProcsMap)
import PostgREST.Error (ApiRequestError (..))
import PostgREST.Query.SqlFragment (ftsOperators, operators)
import PostgREST.RangeQuery (NonnegRange, allRange,
@@ -119,15 +118,15 @@ instance JSON.ToJSON RpcParamValue where
toJSON (Variadic v) = JSON.toJSON v
toRpcParamValue :: ProcDescription -> (Text, Text) -> (Text, RpcParamValue)
toRpcParamValue proc (k, v) | argIsVariadic k = (k, Variadic [v])
toRpcParamValue proc (k, v) | prmIsVariadic k = (k, Variadic [v])
| otherwise = (k, Fixed v)
where
argIsVariadic arg = isJust $ find (\PgArg{pgaName, pgaVar} -> pgaName == arg && pgaVar) $ pdArgs proc
prmIsVariadic prm = isJust $ find (\ProcParam{ppName, ppVar} -> ppName == prm && ppVar) $ pdParams proc
-- | Convert rpc params `/rpc/func?a=val1&b=val2` to json `{"a": "val1", "b": "val2"}
jsonRpcParams :: ProcDescription -> [(Text, Text)] -> PayloadJSON
jsonRpcParams proc prms =
if not $ pdHasVariadic proc then -- if proc has no variadic arg, save steps and directly convert to json
if not $ pdHasVariadic proc then -- if proc has no variadic param, save steps and directly convert to json
ProcessedJSON (JSON.encode $ M.fromList $ second JSON.toJSON <$> prms) (S.fromList $ fst <$> prms)
else
let paramsMap = M.fromListWith mergeParams $ toRpcParamValue proc <$> prms in
@@ -135,7 +134,7 @@ jsonRpcParams proc prms =
where
mergeParams :: RpcParamValue -> RpcParamValue -> RpcParamValue
mergeParams (Variadic a) (Variadic b) = Variadic $ b ++ a
mergeParams v _ = v -- repeated params for non-variadic arguments are not merged
mergeParams v _ = v -- repeated params for non-variadic parameters are not merged
targetToJsonRpcParams :: Maybe Target -> [(Text, Text)] -> Maybe PayloadJSON
targetToJsonRpcParams target params =
@@ -480,37 +479,33 @@ rawContentTypes AppConfig{..} =
(ContentType.decodeContentType <$> configRawMediaTypes) `union` [CTOctetStream, CTTextPlain]
{-|
Search a pg procedure by its parameters. Since a function can be overloaded, the name is not enough to find it.
An overloaded function can have a different volatility or even a different return type.
Search a pg proc by matching name and arguments keys to parameters. Since a function can be overloaded,
the name is not enough to find it. An overloaded function can have a different volatility or even a different return type.
-}
findProc :: QualifiedIdentifier -> S.Set Text -> Bool -> ProcsMap -> Either ApiRequestError ProcDescription
findProc qi payloadKeys paramsAsSingleObject allProcs =
case bestMatch of
[] -> Left $ NoRpc (qiSchema qi) (qiName qi) (S.toList payloadKeys) paramsAsSingleObject
findProc qi argumentsKeys paramsAsSingleObject allProcs =
case matchProc of
[] -> Left $ NoRpc (qiSchema qi) (qiName qi) (S.toList argumentsKeys) paramsAsSingleObject
[proc] -> Right proc
procs -> Left $ AmbiguousRpc (toList procs)
where
bestMatch =
case M.lookup qi allProcs of
Nothing -> []
Just [proc] -> [proc | matches proc]
Just procs -> filter matches procs
-- Find the exact arguments match
matches proc
| paramsAsSingleObject = case pdArgs proc of
[arg] -> pgaType arg `elem` ["json", "jsonb"]
_ -> False
| otherwise = case pdArgs proc of
[] -> null payloadKeys
args -> matchesArg args
matchesArg args =
-- The function's required arguments are separated from the ones with a default value assigned.
-- The set of names of those arguments is compared to the set of keys supplied by the client
-- 1. If only required arguments are found, the keys must be exactly the same as those arguments
-- 2. If only optional arguments are found, the keys must be a subset of those arguments
-- 3. If both required and optional arguments are found, the result of taking away the optional arguments
-- from the keys must be exactly the same as the required arguments
case L.partition pgaReq args of
(reqArgs, []) -> payloadKeys == S.fromList (pgaName <$> reqArgs)
([], defArgs) -> payloadKeys `S.isSubsetOf` S.fromList (pgaName <$> defArgs)
(reqArgs, defArgs) -> payloadKeys `S.difference` S.fromList (pgaName <$> defArgs) == S.fromList (pgaName <$> reqArgs)
matchProc = filter matchesParams $ M.lookupDefault mempty qi allProcs -- first find the proc by name
matchesParams proc =
let params = pdParams proc in
-- here we don't match by argument key(there isn't one) but by the single parameter type
if paramsAsSingleObject then
case params of
[prm] -> ppType prm `elem` ["json", "jsonb"]
_ -> False
-- A function has optional and required parameters. Optional parameters have a default value and
-- don't require arguments for the function to be executed, required parameters must have an argument present.
else case L.partition ppReq params of
-- If the function has no parameters, the arguments keys must be empty as well
([], []) -> null argumentsKeys
-- If the function only has required parameters, the arguments keys must match those parameters
(reqParams, []) -> argumentsKeys == S.fromList (ppName <$> reqParams)
-- If the function only has optional parameters, the arguments keys can match none or any of them(a subset)
([], optParams) -> argumentsKeys `S.isSubsetOf` S.fromList (ppName <$> optParams)
-- If the function has required and optional parameters, the arguments keys have to match the required parameters
-- and can match any or none of the default parameters.
(reqParams, optParams) -> argumentsKeys `S.difference` S.fromList (ppName <$> optParams) == S.fromList (ppName <$> reqParams)
test/Feature/RpcSpec.hs
+8 -8
View File
@@ -107,7 +107,7 @@ spec actualPgVersion =
it "should not ignore unknown args and fail with 404" $
get "/rpc/add_them?a=1&b=2&smthelse=blabla" `shouldRespondWith`
[json| {
"hint":"If a new function was created in the database with this name and arguments, try reloading the schema cache.",
"hint":"If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"message":"Could not find the test.add_them(a, b, smthelse) function in the schema cache" } |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
@@ -119,8 +119,8 @@ spec actualPgVersion =
[json|{}|]
`shouldRespondWith`
[json| {
"hint":"If a new function was created in the database with this name and arguments, try reloading the schema cache.",
"message":"Could not find the test.sayhello function with a single json or jsonb argument in the schema cache" } |]
"hint":"If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"message":"Could not find the test.sayhello function with a single json or jsonb parameter in the schema cache" } |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
@@ -128,24 +128,24 @@ spec actualPgVersion =
it "should fail with 404 for overloaded functions with unknown args" $ do
get "/rpc/overloaded?wrong_arg=value" `shouldRespondWith`
[json| {
"hint":"If a new function was created in the database with this name and arguments, try reloading the schema cache.",
"hint":"If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"message":"Could not find the test.overloaded(wrong_arg) function in the schema cache" } |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
get "/rpc/overloaded?a=1&b=2&wrong_arg=value" `shouldRespondWith`
[json| {
"hint":"If a new function was created in the database with this name and arguments, try reloading the schema cache.",
"hint":"If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"message":"Could not find the test.overloaded(a, b, wrong_arg) function in the schema cache" } |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
context "ambiguous overloaded functions with same arguments but different types" $ do
it "should fail with 300 Multiple Choices without explicit argument type casts" $
context "ambiguous overloaded functions with same parameters' names but different types" $ do
it "should fail with 300 Multiple Choices without explicit type casts" $
get "/rpc/overloaded_same_args?arg=value" `shouldRespondWith`
[json| {
"hint":"Overloaded functions with the same argument name but different types are not supported",
"hint":"Overloaded functions with the same parameter name but different types are not supported",
"message":"Could not choose the best candidate function between: test.overloaded_same_args(arg => integer), test.overloaded_same_args(arg => xml), test.overloaded_same_args(arg => text, num => integer)" } |]
{ matchStatus = 300
, matchHeaders = [matchContentTypeJson]
test/QueryCost.hs
+4 -4
View File
@@ -34,7 +34,7 @@ main = do
context "call proc query" $ do
it "should not exceed cost when calling setof composite proc" $ do
cost <- exec pool $
requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [PgArg "id" "int" True False]
requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [ProcParam "id" "int" True False]
(Just $ RawJSON [str| {"id": 3} |]) False Nothing []
liftIO $
cost `shouldSatisfy` (< Just 40)
@@ -47,7 +47,7 @@ main = do
it "should not exceed cost when calling scalar proc" $ do
cost <- exec pool $
requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [PgArg "a" "int" True False, PgArg "b" "int" True False]
requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [ProcParam "a" "int" True False, ProcParam "b" "int" True False]
(Just $ RawJSON [str| {"a": 3, "b": 4} |]) True Nothing []
liftIO $
cost `shouldSatisfy` (< Just 10)
@@ -55,7 +55,7 @@ main = do
context "params=multiple-objects" $ do
it "should not exceed cost when calling setof composite proc" $ do
cost <- exec pool $
requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [PgArg "id" "int" True False]
requestToCallProcQuery (QualifiedIdentifier "test" "get_projects_below") [ProcParam "id" "int" True False]
(Just $ RawJSON [str| [{"id": 1}, {"id": 4}] |]) False (Just MultipleObjects) []
liftIO $ do
-- lower bound needed for now to make sure that cost is not Nothing
@@ -64,7 +64,7 @@ main = do
it "should not exceed cost when calling scalar proc" $ do
cost <- exec pool $
requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [PgArg "a" "int" True False, PgArg "b" "int" True False]
requestToCallProcQuery (QualifiedIdentifier "test" "add_them") [ProcParam "a" "int" True False, ProcParam "b" "int" True False]
(Just $ RawJSON [str| [{"a": 3, "b": 4}, {"a": 1, "b": 2}, {"a": 8, "b": 7}] |]) True Nothing []
liftIO $
cost `shouldSatisfy` (< Just 10)