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

feat: hint function names/parameters on error

Browse Source
This commit is contained in:
Laurence Isla
2022-12-01 17:17:45 -05:00
committed by GitHub
parent 5e9dba5292
commit 9e567216e9
6 changed files with 104 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files
postgrest.cabal
+1
View File
@@ -85,6 +85,7 @@ library
, contravariant-extras >= 0.3.3 && < 0.4
, cookie >= 0.4.2 && < 0.5
, either >= 4.4.1 && < 5.1
, fuzzyset >= 0.2.3
, gitrev >= 1.2 && < 1.4
, hasql >= 1.6.1.1 && < 1.7
, hasql-dynamic-statements >= 0.3.1 && < 0.4
src/PostgREST/ApiRequest.hs
+4 -2
View File
@@ -453,7 +453,7 @@ requestMediaTypes conf action path =
findProc :: QualifiedIdentifier -> S.Set Text -> Bool -> ProcsMap -> MediaType -> Bool -> Either ApiRequestError ProcDescription
findProc qi argumentsKeys paramsAsSingleObject allProcs contentMediaType isInvPost =
case matchProc of
([], []) -> Left $ NoRpc (qiSchema qi) (qiName qi) (S.toList argumentsKeys) paramsAsSingleObject contentMediaType isInvPost
([], []) -> Left $ NoRpc (qiSchema qi) (qiName qi) (S.toList argumentsKeys) paramsAsSingleObject contentMediaType isInvPost (HM.keys allProcs) lookupProcName
-- If there are no functions with named arguments, fallback to the single unnamed argument function
([], [proc]) -> Right proc
([], procs) -> Left $ AmbiguousRpc (toList procs)
@@ -461,7 +461,9 @@ findProc qi argumentsKeys paramsAsSingleObject allProcs contentMediaType isInvPo
([proc], _) -> Right proc
(procs, _) -> Left $ AmbiguousRpc (toList procs)
where
matchProc = overloadedProcPartition $ HM.lookupDefault mempty qi allProcs -- first find the proc by name
matchProc = overloadedProcPartition lookupProcName
-- First find the proc by name
lookupProcName = HM.lookupDefault mempty qi allProcs
-- The partition obtained has the form (overloadedProcs,fallbackProcs)
-- where fallbackProcs are functions with a single unnamed parameter
overloadedProcPartition = foldr select ([],[])
src/PostgREST/ApiRequest/Types.hs
+3 -2
View File
@@ -32,7 +32,8 @@ module PostgREST.ApiRequest.Types
) where
import PostgREST.MediaType (MediaType (..))
import PostgREST.SchemaCache.Identifiers (FieldName)
import PostgREST.SchemaCache.Identifiers (FieldName,
QualifiedIdentifier)
import PostgREST.SchemaCache.Proc (ProcDescription (..))
import PostgREST.SchemaCache.Relationship (Relationship)
@@ -72,7 +73,7 @@ data ApiRequestError
| LimitNoOrderError
| NotFound
| NoRelBetween Text Text Text
| NoRpc Text Text [Text] Bool MediaType Bool
| NoRpc Text Text [Text] Bool MediaType Bool [QualifiedIdentifier] [ProcDescription]
| NotEmbedded Text
| ParseRequestError Text Text
| PutRangeNotAllowedError
src/PostgREST/Error.hs
+64 -6
View File
@@ -17,6 +17,7 @@ module PostgREST.Error
import qualified Data.Aeson as JSON
import qualified Data.ByteString.Char8 as BS
import qualified Data.FuzzySet as Fuzzy
import qualified Data.Text as T
import qualified Data.Text.Encoding as T
import qualified Data.Text.Encoding.Error as T
@@ -177,26 +178,83 @@ instance JSON.ToJSON ApiRequestError where
"message" .= ("Could not embed because more than one relationship was found for '" <> parent <> "' and '" <> child <> "'" :: Text),
"details" .= (compressedRel <$> rels),
"hint" .= ("Try changing '" <> child <> "' to one of the following: " <> relHint rels <> ". Find the desired relationship in the 'details' key." :: Text)]
toJSON (NoRpc schema procName argumentKeys hasPreferSingleObject contentType isInvPost) =
let prms = "(" <> T.intercalate ", " argumentKeys <> ")" in JSON.object [
toJSON (NoRpc schema procName argumentKeys hasPreferSingleObject contentType isInvPost allProcs overloadedProcs) =
let func = schema <> "." <> procName
prms = "(" <> T.intercalate ", " argumentKeys <> ")"
fmtParams = if null argumentKeys then " function without parameters" else prms <> " function"
in JSON.object [
"code" .= SchemaCacheErrorCode02,
"message" .= ("Could not find the " <> schema <> "." <> procName <>
"message" .= ("Could not find the " <> func <>
(case (hasPreferSingleObject, isInvPost, contentType) of
(True, _, _) -> " function with a single json or jsonb parameter"
(_, True, MTTextPlain) -> " function with a single unnamed text parameter"
(_, True, MTTextXML) -> " function with a single unnamed xml parameter"
(_, True, MTOctetStream) -> " function with a single unnamed bytea parameter"
(_, True, MTApplicationJSON) -> prms <> " function or the " <> schema <> "." <> procName <>" function with a single unnamed json or jsonb parameter"
_ -> prms <> " function") <>
(_, True, MTApplicationJSON) -> fmtParams <> " or the " <> func <>" function with a single unnamed json or jsonb parameter"
_ -> fmtParams) <>
" in the schema cache"),
"details" .= JSON.Null,
"hint" .= ("If a new function was created in the database with this name and parameters, try reloading the schema cache." :: Text)]
-- The hint will be null in the case of single unnamed parameter functions
"hint" .= if hasPreferSingleObject || (isInvPost && contentType `elem` [MTTextPlain, MTTextXML, MTOctetStream])
then Nothing
else noRpcHint schema procName argumentKeys allProcs overloadedProcs ]
toJSON (AmbiguousRpc procs) = JSON.object [
"code" .= SchemaCacheErrorCode03,
"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]),
"details" .= JSON.Null,
"hint" .= ("Try renaming the parameters or the function itself in the database so function overloading can be resolved" :: Text)]
-- |
-- If no function is found with the given name, it does a fuzzy search to all the functions
-- in the same schema and shows the best match as hint.
--
-- >>> :set -Wno-missing-fields
-- >>> let procs = [(QualifiedIdentifier "api" "test"), (QualifiedIdentifier "api" "another"), (QualifiedIdentifier "private" "other")]
--
-- >>> noRpcHint "api" "testt" ["val", "param", "name"] procs []
-- Just "Perhaps you meant to call the function api.test"
--
-- >>> noRpcHint "api" "other" [] procs []
-- Just "Perhaps you meant to call the function api.another"
--
-- >>> noRpcHint "api" "noclosealternative" [] procs []
-- Nothing
--
-- If a function is found with the given name, but no params match, then it does a fuzzy search
-- to all the overloaded functions' params using the form "param1, param2, param3, ..."
-- and shows the best match as hint.
--
-- >>> let procsDesc = [ProcDescription {pdParams = [ProcParam {ppName="val"}, ProcParam {ppName="param"}, ProcParam {ppName="name"}]}, ProcDescription {pdParams = [ProcParam {ppName="id"}, ProcParam {ppName="attr"}]}]
--
-- >>> noRpcHint "api" "test" ["vall", "pqaram", "nam"] procs procsDesc
-- Just "Perhaps you meant to call the function api.test(name, param, val)"
--
-- >>> noRpcHint "api" "test" ["val", "param"] procs procsDesc
-- Just "Perhaps you meant to call the function api.test(name, param, val)"
--
-- >>> noRpcHint "api" "test" ["id", "attrs"] procs procsDesc
-- Just "Perhaps you meant to call the function api.test(attr, id)"
--
-- >>> noRpcHint "api" "test" ["id"] procs procsDesc
-- Just "Perhaps you meant to call the function api.test(attr, id)"
--
-- >>> noRpcHint "api" "test" ["noclosealternative"] procs procsDesc
-- Nothing
--
noRpcHint :: Text -> Text -> [Text] -> [QualifiedIdentifier] -> [ProcDescription] -> Maybe Text
noRpcHint schema procName params allProcs overloadedProcs =
fmap (("Perhaps you meant to call the function " <> schema <> ".") <>) possibleProcs
where
fuzzySetOfProcs = Fuzzy.fromList [qiName k | k <- allProcs, qiSchema k == schema]
fuzzySetOfParams = Fuzzy.fromList $ listToText <$> [[ppName prm | prm <- pdParams ov] | ov <- overloadedProcs]
-- Cannot do a fuzzy search like: Fuzzy.getOne [[Text]] [Text], where [[Text]] is the list of params for each
-- overloaded function and [Text] the given params. This converts those lists to text to make fuzzy search possible.
-- E.g. ["val", "param", "name"] into "(name, param, val)"
listToText = ("(" <>) . (<> ")") . T.intercalate ", " . sort
possibleProcs
| null overloadedProcs = Fuzzy.getOne fuzzySetOfProcs procName
| otherwise = (procName <>) <$> Fuzzy.getOne fuzzySetOfParams (listToText params)
compressedRel :: Relationship -> JSON.Value
-- An ambiguousness error cannot happen for computed relationships TODO refactor so this mempty is not needed
compressedRel ComputedRelationship{} = JSON.object mempty
test/doc/Main.hs
+1
View File
@@ -15,4 +15,5 @@ main =
, "src/PostgREST/Query/SqlFragment.hs"
, "src/PostgREST/ApiRequest/Preferences.hs"
, "src/PostgREST/ApiRequest/QueryParams.hs"
, "src/PostgREST/Error.hs"
]
test/spec/Feature/Query/RpcSpec.hs
+31 -9
View File
@@ -120,14 +120,36 @@ spec actualPgVersion =
it "should fail with 404 on unknown proc name" $
get "/rpc/fake" `shouldRespondWith` 404
it "should fail with 404 and hint the closest proc on unknown proc name" $
get "/rpc/sayhell" `shouldRespondWith`
[json| {
"hint":"Perhaps you meant to call the function test.sayhello",
"message":"Could not find the test.sayhell function without parameters in the schema cache",
"code":"PGRST202",
"details":null} |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
it "should fail with 404 on unknown proc args" $ do
get "/rpc/sayhello" `shouldRespondWith` 404
get "/rpc/sayhello?any_arg=value" `shouldRespondWith` 404
it "should fail with 404 and hint the closest args on unknown proc args" $
get "/rpc/sayhello?nam=Peter" `shouldRespondWith`
[json| {
"hint":"Perhaps you meant to call the function test.sayhello(name)",
"message":"Could not find the test.sayhello(nam) function in the schema cache",
"code":"PGRST202",
"details":null} |]
{ matchStatus = 404
, matchHeaders = [matchContentTypeJson]
}
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 parameters, try reloading the schema cache.",
"hint":"Perhaps you meant to call the function test.add_them(a, b)",
"message":"Could not find the test.add_them(a, b, smthelse) function in the schema cache",
"code":"PGRST202",
"details":null} |]
@@ -141,7 +163,7 @@ spec actualPgVersion =
[json|{}|]
`shouldRespondWith`
[json| {
"hint":"If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"hint":null,
"message":"Could not find the test.sayhello function with a single json or jsonb parameter in the schema cache",
"code":"PGRST202",
"details":null} |]
@@ -152,7 +174,7 @@ 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 parameters, try reloading the schema cache.",
"hint":null,
"message":"Could not find the test.overloaded(wrong_arg) function in the schema cache",
"code":"PGRST202",
"details":null} |]
@@ -161,7 +183,7 @@ spec actualPgVersion =
}
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 parameters, try reloading the schema cache.",
"hint":"Perhaps you meant to call the function test.overloaded(a, b, c)",
"message":"Could not find the test.overloaded(a, b, wrong_arg) function in the schema cache",
"code":"PGRST202",
"details":null} |]
@@ -1251,7 +1273,7 @@ spec actualPgVersion =
[json|{"x": 1, "y": 2}|]
`shouldRespondWith`
[json|{
"hint": "If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"hint": "Perhaps you meant to call the function test.unnamed_text_param",
"message": "Could not find the test.unnamed_int_param(x, y) function or the test.unnamed_int_param function with a single unnamed json or jsonb parameter in the schema cache",
"code":"PGRST202",
"details":null
@@ -1266,7 +1288,7 @@ spec actualPgVersion =
[str|a simple text|]
`shouldRespondWith`
[json|{
"hint": "If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"hint": null,
"message": "Could not find the test.unnamed_int_param function with a single unnamed text parameter in the schema cache",
"code":"PGRST202",
"details":null
@@ -1281,7 +1303,7 @@ spec actualPgVersion =
[str|a simple text|]
`shouldRespondWith`
[json|{
"hint": "If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"hint": null,
"message": "Could not find the test.unnamed_int_param function with a single unnamed xml parameter in the schema cache",
"code":"PGRST202",
"details":null
@@ -1297,7 +1319,7 @@ spec actualPgVersion =
file
`shouldRespondWith`
[json|{
"hint": "If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"hint": null,
"message": "Could not find the test.unnamed_int_param function with a single unnamed bytea parameter in the schema cache",
"code":"PGRST202",
"details":null
@@ -1357,7 +1379,7 @@ spec actualPgVersion =
"a,b\n1,2\n4,6\n100,200"
`shouldRespondWith`
[json| {
"hint":"If a new function was created in the database with this name and parameters, try reloading the schema cache.",
"hint":"Perhaps you meant to call the function test.overloaded_unnamed_param(x, y)",
"message":"Could not find the test.overloaded_unnamed_param(a, b) function in the schema cache",
"code":"PGRST202",
"details":null