From 47b023e85852249900854668781f031083dfbb3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steve=20Ch=C3=A1vez?= Date: Tue, 21 Mar 2017 01:50:42 -0500 Subject: [PATCH] Fix proc resource embedding issue with search_path and refactor return type (#831) --- CHANGELOG.md | 1 + src/PostgREST/App.hs | 5 +-- src/PostgREST/DbRequestBuilder.hs | 18 ++++----- src/PostgREST/DbStructure.hs | 38 +++++++++++++----- src/PostgREST/Types.hs | 9 ++++- test/Feature/QuerySpec.hs | 65 +++++++++++++++++++++++++++---- test/fixtures/schema.sql | 39 +++++++++++++++++++ 7 files changed, 143 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ef53efb7..6d62b043f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #791, malformed nested JSON error - @diogob - Resource embedding in views referencing tables in public schema - @fab1an +- #831, Fix proc resource embedding issue with search_path - @steve-chavez ## [0.4.0.0] - 2017-01-19 diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 76cfff275..ba3c37579 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -232,7 +232,7 @@ app dbStructure conf apiRequest = uri Nothing = ("http", host, port, "/") uri (Just Proxy { proxyScheme = s, proxyHost = h, proxyPort = p, proxyPath = b }) = (s, h, p, b) uri' = uri proxy - encodeApi ti = encodeOpenAPI (map snd $ dbProcs dbStructure) ti uri' + encodeApi ti = encodeOpenAPI (M.elems $ dbProcs dbStructure) ti uri' body <- encodeApi . toTableInfo <$> H.query schema accessibleTables return $ responseLBS status200 [toHeader CTOpenAPI] $ toS body @@ -264,8 +264,7 @@ app dbStructure conf apiRequest = status = rangeStatus lower upper (toInteger <$> tableTotal) in (status, contentRange) - mapSnd f (a, b) = (a, f b) - readReq = readRequest (configMaxRows conf) (dbRelations dbStructure) (map (mapSnd pdReturnType) $ dbProcs dbStructure) apiRequest + readReq = readRequest (configMaxRows conf) (dbRelations dbStructure) (dbProcs dbStructure) apiRequest fldNames = fieldNames <$> readReq readDbRequest = DbRead <$> readReq mutateDbRequest = DbMutate <$> (mutateRequest apiRequest =<< fldNames) diff --git a/src/PostgREST/DbRequestBuilder.hs b/src/PostgREST/DbRequestBuilder.hs index 0e276b694..61e701fad 100644 --- a/src/PostgREST/DbRequestBuilder.hs +++ b/src/PostgREST/DbRequestBuilder.hs @@ -9,9 +9,9 @@ import Control.Applicative import Control.Lens.Getter (view) import Control.Lens.Tuple (_1) import qualified Data.ByteString.Char8 as BS -import Data.List (delete, lookup) +import Data.List (delete) import Data.Maybe (fromJust) -import Data.Text (isInfixOf, dropWhile, drop) +import Data.Text (isInfixOf) import Data.Tree import Data.Either.Combinators (mapLeft) @@ -35,7 +35,7 @@ import Protolude hiding (from, dropWhile, drop) import Text.Regex.TDFA ((=~)) import Unsafe (unsafeHead) -readRequest :: Maybe Integer -> [Relation] -> [(Text, Text)] -> ApiRequest -> Either Response ReadRequest +readRequest :: Maybe Integer -> [Relation] -> M.HashMap Text ProcDescription -> ApiRequest -> Either Response ReadRequest readRequest maxRows allRels allProcs apiRequest = mapLeft apiRequestError $ treeRestrictRange maxRows =<< @@ -46,13 +46,13 @@ readRequest maxRows allRels allProcs apiRequest = let target = iTarget apiRequest in case target of (TargetIdent (QualifiedIdentifier s t) ) -> Just (s, t) - (TargetProc (QualifiedIdentifier s p) ) -> Just (s, t) + (TargetProc (QualifiedIdentifier s proc) ) -> Just (s, tName) where - returnType = fromMaybe "" $ lookup p allProcs - -- we are looking for results looking like "SETOF schema.tablename" and want to extract tablename - t = if "SETOF " `isInfixOf` returnType - then drop 1 $ dropWhile (/= '.') returnType - else p + retType = pdReturnType <$> M.lookup proc allProcs + tName = case retType of + Just (SetOf (Composite qi)) -> qiName qi + Just (Single (Composite qi)) -> qiName qi + _ -> proc _ -> Nothing diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index 135e589c2..1bc2142ca 100644 --- a/src/PostgREST/DbStructure.hs +++ b/src/PostgREST/DbStructure.hs @@ -13,6 +13,7 @@ import qualified Hasql.Encoders as HE import qualified Hasql.Query as H import Control.Applicative +import qualified Data.HashMap.Strict as M import Data.List (elemIndex) import Data.Maybe (fromJust) import Data.Text (split, strip, @@ -96,12 +97,13 @@ decodeSynonyms cols = <*> HD.value HD.text <*> HD.value HD.text <*> HD.value HD.text <*> HD.value HD.text -accessibleProcs :: H.Query Schema [(Text, ProcDescription)] +accessibleProcs :: H.Query Schema (M.HashMap Text ProcDescription) accessibleProcs = H.statement sql (HE.value HE.text) - (map addName <$> HD.rowsList (ProcDescription <$> HD.value HD.text + (M.fromList . map addName <$> HD.rowsList (ProcDescription <$> HD.value HD.text <*> (parseArgs <$> HD.value HD.text) - <*> HD.value HD.text)) True + <*> (parseRetType <$> HD.value HD.text <*> HD.value HD.text <*> + HD.value HD.bool <*> HD.value HD.char))) True where addName :: ProcDescription -> (Text, ProcDescription) addName pd = (pdName pd, pd) @@ -118,14 +120,30 @@ accessibleProcs = else Just $ PgArg (dropAround (== '"') name) (strip typ) (T.null def) + parseRetType :: Text -> Text -> Bool -> Char -> RetType + parseRetType schema name isSetOf typ + | isSetOf = SetOf pgType + | otherwise = Single pgType + where + qi = QualifiedIdentifier schema name + pgType = case typ of + 'c' -> Composite qi + 'p' -> Pseudo name + _ -> Scalar qi -- 'b'ase, 'd'omain, 'e'num, 'r'ange + sql = [q| - SELECT p.proname as "proc_name", - pg_get_function_arguments(p.oid) as "args", - pg_get_function_result(p.oid) as "return_type" - FROM pg_namespace n - JOIN pg_proc p - ON pronamespace = n.oid - WHERE n.nspname = $1|] + SELECT p.proname as "proc_name", + pg_get_function_arguments(p.oid) as "args", + tn.nspname as "rettype_schema", + coalesce(comp.relname, t.typname) as "rettype_name", + p.proretset as "rettype_is_setof", + t.typtype as "rettype_typ" + FROM pg_proc p + JOIN pg_namespace pn ON pn.oid = p.pronamespace + JOIN pg_type t ON t.oid = p.prorettype + JOIN pg_namespace tn ON tn.oid = t.typnamespace + LEFT JOIN pg_class comp ON comp.oid = t.typrelid + WHERE pn.nspname = $1|] accessibleTables :: H.Query Schema [Table] accessibleTables = diff --git a/src/PostgREST/Types.hs b/src/PostgREST/Types.hs index d3160db0a..4895aab7e 100644 --- a/src/PostgREST/Types.hs +++ b/src/PostgREST/Types.hs @@ -3,6 +3,7 @@ import Protolude import qualified GHC.Show import Data.Aeson import qualified Data.ByteString.Lazy as BL +import Data.HashMap.Strict as M import Data.Tree import qualified Data.Vector as V import PostgREST.RangeQuery (NonnegRange) @@ -27,7 +28,7 @@ data DbStructure = DbStructure { , dbColumns :: [Column] , dbRelations :: [Relation] , dbPrimaryKeys :: [PrimaryKey] -, dbProcs :: [(Text,ProcDescription)] +, dbProcs :: M.HashMap Text ProcDescription } deriving (Show, Eq) data PgArg = PgArg { @@ -36,10 +37,14 @@ data PgArg = PgArg { , pgaReq :: Bool } deriving (Show, Eq) +data PgType = Scalar QualifiedIdentifier | Composite QualifiedIdentifier | Pseudo Text deriving (Eq, Show) + +data RetType = Single PgType | SetOf PgType deriving (Eq, Show) + data ProcDescription = ProcDescription { pdName :: Text , pdArgs :: [PgArg] -, pdReturnType :: Text +, pdReturnType :: RetType } deriving (Show, Eq) type Schema = Text diff --git a/test/Feature/QuerySpec.hs b/test/Feature/QuerySpec.hs index 32648dbd6..f1fe446d2 100644 --- a/test/Feature/QuerySpec.hs +++ b/test/Feature/QuerySpec.hs @@ -512,25 +512,74 @@ spec = do post "/rpc/getproject?select=id,name" [json| { "id": 1} |] `shouldRespondWith` [str|[{"id":1,"name":"Windows 7"}]|] - it "can embed foreign entities to the items returned by a proc" $ + context "foreign entities embedding" $ do + it "can embed if related tables are in the exposed schema" $ post "/rpc/getproject?select=id,name,client{id},tasks{id}" [json| { "id": 1} |] `shouldRespondWith` [str|[{"id":1,"name":"Windows 7","client":{"id":1},"tasks":[{"id":1},{"id":2}]}]|] + it "cannot embed if the related table is not in the exposed schema" $ + post "/rpc/single_article?select=*,article_stars{*}" [json|{ "id": 1}|] + `shouldRespondWith` 400 + + it "can embed if the related tables are in a hidden schema but exposed as views" $ + post "/rpc/single_article?select=id,articleStars{userId}" [json|{ "id": 2}|] + `shouldRespondWith` [json|[{"id": 2, "articleStars": [{"userId": 3}]}]|] + { matchHeaders = [matchContentTypeJson] } + context "a proc that returns an empty rowset" $ it "returns empty json array" $ post "/rpc/test_empty_rowset" [json| {} |] `shouldRespondWith` [json| [] |] { matchHeaders = [matchContentTypeJson] } - context "a proc that returns plain text" $ do - it "returns proper json" $ - post "/rpc/sayhello" [json| { "name": "world" } |] `shouldRespondWith` - [json|"Hello, world"|] + context "proc return types" $ do + context "returns text" $ do + it "returns proper json" $ + post "/rpc/sayhello" [json| { "name": "world" } |] `shouldRespondWith` + [json|"Hello, world"|] + { matchHeaders = [matchContentTypeJson] } + + it "can handle unicode" $ + post "/rpc/sayhello" [json| { "name": "￥" } |] `shouldRespondWith` + [json|"Hello, ￥"|] + { matchHeaders = [matchContentTypeJson] } + + it "returns enum value" $ + post "/rpc/ret_enum" [json|{ "val": "foo" }|] `shouldRespondWith` + [json|"foo"|] { matchHeaders = [matchContentTypeJson] } - it "can handle unicode" $ - post "/rpc/sayhello" [json| { "name": "￥" } |] `shouldRespondWith` - [json|"Hello, ￥"|] + it "returns domain value" $ + post "/rpc/ret_domain" [json|{ "val": "8" }|] `shouldRespondWith` + [json|8|] + { matchHeaders = [matchContentTypeJson] } + + it "returns range" $ + post "/rpc/ret_range" [json|{ "low": 10, "up": 20 }|] `shouldRespondWith` + [json|"[10,20)"|] + { matchHeaders = [matchContentTypeJson] } + + it "returns row of scalars" $ + post "/rpc/ret_scalars" [json|{}|] `shouldRespondWith` + [json|[{"a":"scalars", "b":"foo", "c":1, "d":"[10,20)"}]|] + { matchHeaders = [matchContentTypeJson] } + + it "returns composite type in exposed schema" $ + post "/rpc/ret_point_2d" [json|{}|] `shouldRespondWith` + [json|[{"x": 10, "y": 5}]|] + { matchHeaders = [matchContentTypeJson] } + + it "cannot return composite type in hidden schema" $ + post "/rpc/ret_point_3d" [json|{}|] `shouldRespondWith` 401 + + it "returns single row from table" $ + post "/rpc/single_article?select=id" [json|{"id": 2}|] `shouldRespondWith` + [json|[{"id": 2}]|] + { matchHeaders = [matchContentTypeJson] } + + it "returns null for void" $ + post "/rpc/ret_void" [json|{}|] `shouldRespondWith` + [json|null|] { matchHeaders = [matchContentTypeJson] } context "improper input" $ do diff --git a/test/fixtures/schema.sql b/test/fixtures/schema.sql index 11c96cc64..b06b68fe8 100755 --- a/test/fixtures/schema.sql +++ b/test/fixtures/schema.sql @@ -1108,6 +1108,45 @@ create view images_base64 as ( select name, replace(encode(img, 'base64'), E'\n', '') as img from images ); +create function test.ret_enum(val text) returns test.enum_menagerie_type as $$ + select val::test.enum_menagerie_type; +$$ language sql; + +create domain one_nine as integer check (value >= 1 and value <= 9); + +create function test.ret_domain(val integer) returns test.one_nine as $$ + select val::test.one_nine; +$$ language sql; + +create function test.ret_range(low integer, up integer) returns int4range as $$ + select int4range(low, up); +$$ language sql; + +create function test.ret_scalars() returns table( + a text, b test.enum_menagerie_type, c test.one_nine, d int4range +) as $$ + select row('scalars'::text, enum_first(null::test.enum_menagerie_type), + 1::test.one_nine, int4range(10, 20)); +$$ language sql; + +create type test.point_2d as (x integer, y integer); + +create function test.ret_point_2d() returns test.point_2d as $$ + select row(10, 5)::test.point_2d; +$$ language sql; + +create type private.point_3d as (x integer, y integer, z integer); + +create function test.ret_point_3d() returns private.point_3d as $$ + select row(7, -3, 4)::private.point_3d; +$$ language sql; + +create function test.ret_void() returns void as '' language sql; + +create function test.single_article(id integer) returns test.articles as $$ + select a.* from test.articles a where a.id = $1; +$$ language sql; + -- -- PostgreSQL database dump complete --