Merge pull request #417 from ruslantalpa/fix_399_remove_returning

Fix #399 insert records in tables with no SELECT privileges

CHANGELOG.md

@@ -14,6 +14,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - `pgFmtIdent` always quotes #388 - @calebmer
 - Default schema, changed from `"1"` to `public` - @calebmer
 - #414 revert to separate count query
+- Fix #399, allow inserting in tables with no select privileges using "Prefer: representation=minimal" - @ruslantalpa
 
 ### Added
 - Allow order by computed columns - @diogob

src/PostgREST/ApiRequest.hs

@@ -33,6 +33,8 @@ data Action = ActionCreate | ActionRead
 data Target = TargetIdent QualifiedIdentifier
             | TargetRoot
             | TargetUnknown [T.Text]
+-- | How to return the inserted data
+data PreferRepresentation = Full | HeadersOnly | None deriving Eq
 -- | Enumeration of currently supported content types for
 -- route responses and upload payloads
 data ContentType = ApplicationJSON | TextCSV deriving Eq
@@ -59,7 +61,7 @@ data ApiRequest = ApiRequest {
   -- | Data sent by client and used for mutation actions
   , iPayload :: Maybe Payload
   -- | If client wants created items echoed back
-  , iPreferRepresentation :: Bool
+  , iPreferRepresentation :: PreferRepresentation
   -- | If client wants first row as raw object
   , iPreferSingular :: Bool
   -- | Whether the client wants a result count (slower)
@@ -119,7 +121,7 @@ userApiRequest schema req reqBody =
   , iTarget = target
   , iAccepts = pickContentType $ lookupHeader "accept"
   , iPayload = relevantPayload
-  , iPreferRepresentation = hasPrefer "return=representation"
+  , iPreferRepresentation = representation
   , iPreferSingular = singular
   , iPreferCount = not $ hasPrefer "count=none"
   , iFilters = [ (k, fromJust v) | (k,v) <- qParams, k `notElem` ["select", "order"], isJust v ]
@@ -138,8 +140,10 @@ userApiRequest schema req reqBody =
   lookupHeader    = flip lookup hdrs
   hasPrefer val   = any (\(h,v) -> h == "Prefer" && v == val) hdrs
   singular        = hasPrefer "plurality=singular"
-
-
+  representation
+    | hasPrefer "return=representation" = Full
+    | hasPrefer "return=minimal" = None
+    | otherwise = HeadersOnly
 
 -- PRIVATE ---------------------------------------------------------------
 

src/PostgREST/App.hs

@@ -43,6 +43,7 @@ import           PostgREST.DbStructure
 import           PostgREST.RangeQuery
 import           PostgREST.ApiRequest   (ApiRequest(..), ContentType(..)
                                             , Action(..), Target(..)
+                                            , PreferRepresentation (..)
                                             , userApiRequest)
 import           PostgREST.Types
 import           PostgREST.Auth            (tokenJWT)
@@ -105,14 +106,14 @@ app dbStructure conf reqBody req =
                   )
                 ] (fromMaybe "[]" body)
 
-    (ActionCreate, TargetIdent (QualifiedIdentifier _ table),
+    (ActionCreate, TargetIdent qi@(QualifiedIdentifier _ table),
      Just payload@(PayloadJSON (UniformObjects rows))) ->
       case mutateSqlParts of
         Left e -> return $ responseLBS status400 [jsonH] $ cs e
         Right (sq,mq) -> do
           let isSingle = (==1) $ V.length rows
           let pKeys = map pkName $ filter (filterPk schema table) allPrKeys -- would it be ok to move primary key detection in the query itself?
-          let stm = createWriteStatement sq mq isSingle (iPreferRepresentation apiRequest) pKeys (contentType == TextCSV) payload
+          let stm = createWriteStatement qi sq mq isSingle (iPreferRepresentation apiRequest) pKeys (contentType == TextCSV) payload
           row <- H.maybeEx stm
           let (_, _, location, body) = extractQueryResult row
           return $ responseLBS status201
@@ -120,28 +121,28 @@ app dbStructure conf reqBody req =
               contentTypeH,
               (hLocation, "/" <> cs table <> "?" <> cs (fromMaybe "" location))
             ]
-            $ if iPreferRepresentation apiRequest then fromMaybe "[]" body else ""
+            $ if iPreferRepresentation apiRequest == Full then fromMaybe "[]" body else ""
 
-    (ActionUpdate, TargetIdent _, Just payload@(PayloadJSON _)) ->
+    (ActionUpdate, TargetIdent qi, Just payload@(PayloadJSON _)) ->
       case mutateSqlParts of
         Left e -> return $ responseLBS status400 [jsonH] $ cs e
         Right (sq,mq) -> do
-          let stm = createWriteStatement sq mq False (iPreferRepresentation apiRequest) [] (contentType == TextCSV) payload
+          let stm = createWriteStatement qi sq mq False (iPreferRepresentation apiRequest) [] (contentType == TextCSV) payload
           row <- H.maybeEx stm
           let (_, queryTotal, _, body) = extractQueryResult row
               r = contentRangeH 0 (queryTotal-1) (Just queryTotal)
               s = case () of _ | queryTotal == 0 -> status404
-                               | iPreferRepresentation apiRequest -> status200
+                               | iPreferRepresentation apiRequest == Full -> status200
                                | otherwise -> status204
           return $ responseLBS s [contentTypeH, r]
-            $ if iPreferRepresentation apiRequest then fromMaybe "[]" body else ""
+            $ if iPreferRepresentation apiRequest == Full then fromMaybe "[]" body else ""
 
-    (ActionDelete, TargetIdent _, Nothing) ->
+    (ActionDelete, TargetIdent qi, Nothing) ->
       case mutateSqlParts of
         Left e -> return $ responseLBS status400 [jsonH] $ cs e
         Right (sq,mq) -> do
           let fakeload = PayloadJSON $ UniformObjects V.empty
-          let stm = createWriteStatement sq mq False False [] (contentType == TextCSV) fakeload
+          let stm = createWriteStatement qi sq mq False (iPreferRepresentation apiRequest) [] (contentType == TextCSV) fakeload
           row <- H.maybeEx stm
           let (_, queryTotal, _, _) = extractQueryResult row
           return $ if queryTotal == 0

src/PostgREST/QueryBuilder.hs

@@ -52,6 +52,7 @@ import           Data.Scientific         ( FPFormat (..)
                                          , isInteger
                                          )
 import           Prelude hiding          (unwords)
+import           PostgREST.ApiRequest    (PreferRepresentation (..))
 
 type PStmt = H.Stmt P.Postgres
 instance Monoid PStmt where
@@ -79,23 +80,40 @@ createReadStatement selectQuery countQuery range isSingle countTotal asCsv =
       | isSingle = asJsonSingleF
       | otherwise = asJsonF
 
-createWriteStatement :: SqlQuery -> SqlQuery -> Bool -> Bool ->
+createWriteStatement :: QualifiedIdentifier -> SqlQuery -> SqlQuery -> Bool -> PreferRepresentation ->
                         [Text] -> Bool -> Payload -> B.Stmt P.Postgres
-createWriteStatement _ _ _ _ _ _ (PayloadParseError _) = undefined
-createWriteStatement selectQuery mutateQuery isSingle echoRequested
-                     pKeys asCsv (PayloadJSON (UniformObjects rows)) =
+createWriteStatement _ _ _ _ _ _ _ (PayloadParseError _) = undefined
+createWriteStatement _ _ mutateQuery _ None
+                     _ _ (PayloadJSON (UniformObjects rows)) =
   B.Stmt (
     "WITH " <> sourceCTEName <> " AS (" <> mutateQuery <> ") " <>
+    "SELECT null, 0, null, null"
+  ) (V.singleton . B.encodeValue . JSON.Array . V.map JSON.Object $ rows) True
+createWriteStatement qi _ mutateQuery isSingle HeadersOnly
+                     pKeys _ (PayloadJSON (UniformObjects rows)) =
+  B.Stmt (
+    "WITH " <> sourceCTEName <> " AS (" <> mutateQuery <> " RETURNING " <> fromQi qi <> ".*" <> ") " <>
+    "SELECT " <> intercalate ", " [
+      "null AS total_result_set",
+      "pg_catalog.count(t) AS page_total",
+      if isSingle then locationF pKeys else "null",
+      "null"
+    ] <>
+    " FROM (SELECT 1 FROM " <> sourceCTEName <> ") t"
+  ) (V.singleton . B.encodeValue . JSON.Array . V.map JSON.Object $ rows) True
+createWriteStatement qi selectQuery mutateQuery isSingle Full
+                     pKeys asCsv (PayloadJSON (UniformObjects rows)) =
+  B.Stmt (
+    "WITH " <> sourceCTEName <> " AS (" <> mutateQuery <> " RETURNING " <> fromQi qi <> ".*" <> ") " <>
     "SELECT " <> intercalate ", " [
       "null AS total_result_set", -- when updateing it does not make sense
       "pg_catalog.count(t) AS page_total",
-      location <> " AS header",
-      (if echoRequested then bodyF else "null") <> " AS body"
+      if isSingle then locationF pKeys else "null" <> " AS header",
+      bodyF <> " AS body"
     ] <>
     " FROM ( "<>selectQuery<>") t"
   ) (V.singleton . B.encodeValue . JSON.Array . V.map JSON.Object $ rows) True
   where
-    location = if isSingle then locationF pKeys else "null"
     bodyF
       | asCsv = asCsvF
       | isSingle = asJsonSingleF
@@ -270,8 +288,7 @@ requestToQuery schema (DbMutate (Insert mainTbl (PayloadJSON (UniformObjects row
     "INSERT INTO ", fromQi qi,
     " (" <> colsString <> ")" <>
     " SELECT " <> colsString <>
-    " FROM json_populate_recordset(null::" , fromQi qi, ", ?)",
-    " RETURNING " <> fromQi qi <> ".*"
+    " FROM json_populate_recordset(null::" , fromQi qi, ", ?)"
     ]
 requestToQuery schema (DbMutate (Update mainTbl (PayloadJSON (UniformObjects rows)) conditions)) =
   case rows V.!? 0 of
@@ -281,8 +298,7 @@ requestToQuery schema (DbMutate (Update mainTbl (PayloadJSON (UniformObjects row
       unwords [
         "UPDATE ", fromQi qi,
         " SET " <> intercalate "," assignments <> " ",
-        ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions,
-        "RETURNING " <> fromQi qi <> ".*"
+        ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions
         ]
     Nothing -> undefined
   where
@@ -294,8 +310,7 @@ requestToQuery schema (DbMutate (Delete mainTbl conditions)) =
     qi = QualifiedIdentifier schema mainTbl
     query = unwords [
       "DELETE FROM ", fromQi qi,
-      ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions,
-      "RETURNING " <> fromQi qi <> ".*"
+      ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions
       ]
 
 sourceCTEName :: SqlFragment

test/Feature/InsertSpec.hs

@@ -93,6 +93,15 @@ spec struct pool = beforeAll_ resetDb $ around (withApp cfgDefault struct pool)
             simpleHeaders p `shouldSatisfy` matchHeader hLocation "/no_pk\\?a=eq.bar&b=eq.baz"
             simpleStatus p `shouldBe` created201
 
+        it "can insert in tables with no select privileges" $ do
+          p <- request methodPost "/insertonly"
+                       [("Prefer", "return=minimal")]
+                       [json| { "v":"some value" } |]
+          liftIO $ do
+            simpleBody p `shouldBe` ""
+            simpleStatus p `shouldBe` created201
+
+
         it "can post nulls" $ do
           p <- request methodPost "/no_pk"
                        [("Prefer", "return=representation")]

test/Feature/StructureSpec.hs

@@ -28,6 +28,7 @@ spec struct pool = around (withApp cfgDefault struct pool) $ do
         , {"schema":"test","name":"has_count_column","insertable":false}
         , {"schema":"test","name":"has_fk","insertable":true}
         , {"schema":"test","name":"insertable_view_with_join","insertable":true}
+        , {"schema":"test","name":"insertonly","insertable":true}
         , {"schema":"test","name":"items","insertable":true}
         , {"schema":"test","name":"json","insertable":true}
         , {"schema":"test","name":"materialized_view","insertable":false}

test/fixtures/privileges.sql

@@ -34,6 +34,8 @@ GRANT ALL ON TABLE
     , users_tasks
 TO postgrest_test_anonymous;
 
+GRANT INSERT ON TABLE insertonly TO postgrest_test_anonymous;
+
 GRANT USAGE ON SEQUENCE
       auto_incrementing_pk_id_seq
     , items_id_seq

test/fixtures/schema.sql

@@ -500,6 +500,15 @@ CREATE TABLE nullable_integer (
 );
 
 
+--
+-- Name: insertonly; Type: TABLE; Schema: test; Owner: -
+--
+
+CREATE TABLE insertonly (
+    v text NOT NULL
+);
+
+
 --
 -- Name: projects; Type: TABLE; Schema: test; Owner: -
 --