diff --git a/CHANGELOG.md b/CHANGELOG.md index cdbb5c917..9734e7804 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). - #2156, Allow applying `limit/offset` to UPDATE/DELETE to only affect a subset of rows - @steve-chavez + Uses the table primary key, so it needs a select privilege on the primary key columns + If no primary key is available, it will fallback to using the "ctid" system column(will also require a select privilege on it) - + Will work on views if the PK(or "ctid") is present on its SELECT clause + + Doesn't work on views and it will throw an error if tried ### Fixed @@ -50,6 +50,7 @@ This project adheres to [Semantic Versioning](http://semver.org/). + Previously, those RPCs would return "null" as a body with Content-Type: application/json. - #2156, `limit/offset` now limits the affected rows on UPDATE/DELETE - @steve-chavez + Previously, `limit/offset` only limited the returned rows but not the actual updated rows + - #2156, using PATCH/DELETE with `limit/offset` throws an error on views - @steve-chavez - #2155, `max-rows` is no longer applied on POST/PATCH/PUT/DELETE returned rows - @steve-chavez + This was misleading because the affected rows were not really affected by `max-rows`, only the returned rows were limited diff --git a/src/PostgREST/App.hs b/src/PostgREST/App.hs index 1470011f2..334a87116 100644 --- a/src/PostgREST/App.hs +++ b/src/PostgREST/App.hs @@ -63,7 +63,7 @@ import PostgREST.Config (AppConfig (..), import PostgREST.Config.PgVersion (PgVersion (..)) import PostgREST.ContentType (ContentType (..)) import PostgREST.DbStructure (DbStructure (..), - findTable, + findIfView, findTable, tablePKCols) import PostgREST.DbStructure.Identifiers (FieldName, QualifiedIdentifier (..), @@ -346,7 +346,10 @@ handleCreate identifier@QualifiedIdentifier{..} context@RequestContext{..} = do response HTTP.status201 headers mempty handleUpdate :: QualifiedIdentifier -> RequestContext -> DbHandler Wai.Response -handleUpdate identifier context@(RequestContext _ _ ApiRequest{..} _) = do +handleUpdate identifier context@(RequestContext _ ctxDbStructure ApiRequest{..} _) = do + when (iTopLevelRange /= RangeQuery.allRange && findIfView identifier (dbTables ctxDbStructure)) $ + throwError $ Error.NotImplemented "limit/offset is not implemented for views" + WriteQueryResult{..} <- writeQuery MutationUpdate identifier False mempty context let @@ -392,7 +395,10 @@ handleSingleUpsert identifier context@(RequestContext _ _ ApiRequest{..} _) = do response HTTP.status204 [] mempty handleDelete :: QualifiedIdentifier -> RequestContext -> DbHandler Wai.Response -handleDelete identifier context@(RequestContext _ _ ApiRequest{..} _) = do +handleDelete identifier context@(RequestContext _ ctxDbStructure ApiRequest{..} _) = do + when (iTopLevelRange /= RangeQuery.allRange && findIfView identifier (dbTables ctxDbStructure)) $ + throwError $ Error.NotImplemented "limit/offset is not implemented for views" + WriteQueryResult{..} <- writeQuery MutationDelete identifier False mempty context let diff --git a/src/PostgREST/DbStructure.hs b/src/PostgREST/DbStructure.hs index fcb08b9de..7bbc92b62 100644 --- a/src/PostgREST/DbStructure.hs +++ b/src/PostgREST/DbStructure.hs @@ -23,6 +23,7 @@ module PostgREST.DbStructure , queryDbStructure , accessibleTables , accessibleProcs + , findIfView , findTable , schemaDescription , tableCols @@ -80,7 +81,10 @@ tablePKCols :: DbStructure -> Schema -> TableName -> [Text] tablePKCols dbs tSchema tName = pkName <$> filter (\pk -> tSchema == (tableSchema . pkTable) pk && tName == (tableName . pkTable) pk) (dbPrimaryKeys dbs) findTable :: Schema -> TableName -> [Table] -> Maybe Table -findTable tSchema tName tbls = find (\tbl -> tableName tbl == tName && tableSchema tbl == tSchema) tbls +findTable tSchema tName = find (\tbl -> tableSchema tbl == tSchema && tableName tbl == tName) + +findIfView :: QualifiedIdentifier -> [Table] -> Bool +findIfView identifier tbls = maybe False tableIsView (findTable (qiSchema identifier) (qiName identifier) tbls) -- | The source table column a view column refers to type SourceColumn = (Column, ViewColumn) @@ -138,6 +142,7 @@ decodeTables = <*> column HD.bool <*> column HD.bool <*> column HD.bool + <*> column HD.bool decodeColumns :: [Table] -> HD.Result [Column] decodeColumns tables = @@ -337,6 +342,7 @@ accessibleTables pgVer = n.nspname as table_schema, relname as table_name, d.description as table_description, + c.relkind IN ('v','m') as is_view, ( c.relkind IN ('r','p') OR ( @@ -472,6 +478,7 @@ allTables pgVer = n.nspname AS table_schema, c.relname AS table_name, d.description AS table_description, + c.relkind IN ('v','m') as is_view, ( c.relkind IN ('r','p') OR ( diff --git a/src/PostgREST/DbStructure/Table.hs b/src/PostgREST/DbStructure/Table.hs index 2dab546df..fa6c77c12 100644 --- a/src/PostgREST/DbStructure/Table.hs +++ b/src/PostgREST/DbStructure/Table.hs @@ -20,6 +20,8 @@ data Table = Table { tableSchema :: Schema , tableName :: TableName , tableDescription :: Maybe Text + -- TODO Find a better way to separate tables and views + , tableIsView :: Bool -- The following fields identify what can be done on the table/view, they're not related to the privileges granted to it , tableInsertable :: Bool , tableUpdatable :: Bool diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 60880c1ef..8c5a5632b 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -314,6 +314,7 @@ data Error | JwtTokenInvalid Text | JwtTokenMissing | JwtTokenRequired + | NotImplemented Text | NoSchemaCacheError | NotFound | PgErr PgError @@ -335,6 +336,7 @@ instance PgrstError Error where status (PgErr err) = status err status PutMatchingPkError = HTTP.status400 status PutRangeNotAllowedError = HTTP.status400 + status (NotImplemented _) = HTTP.status501 status SingularityError{} = HTTP.status406 status UnsupportedVerb{} = HTTP.status405 @@ -407,6 +409,12 @@ instance JSON.ToJSON Error where "details" .= JSON.Null, "hint" .= JSON.Null] + toJSON (NotImplemented msg) = JSON.object [ + "code" .= GeneralErrorCode07, + "message" .= msg, + "details" .= JSON.Null, + "hint" .= JSON.Null] + toJSON NotFound = JSON.object [] toJSON (PgErr err) = JSON.toJSON err toJSON (ApiRequestError err) = JSON.toJSON err @@ -460,6 +468,7 @@ data ErrorCode | GeneralErrorCode04 | GeneralErrorCode05 | GeneralErrorCode06 + | GeneralErrorCode07 instance JSON.ToJSON ErrorCode where toJSON e = JSON.toJSON (buildErrorCode e) @@ -504,3 +513,4 @@ buildErrorCode code = "PGRST" <> case code of GeneralErrorCode04 -> "504" GeneralErrorCode05 -> "505" GeneralErrorCode06 -> "506" + GeneralErrorCode07 -> "507" diff --git a/test/spec/Feature/OpenApi/RootSpec.hs b/test/spec/Feature/OpenApi/RootSpec.hs index 6d9bbe15a..3baa3bc77 100644 --- a/test/spec/Feature/OpenApi/RootSpec.hs +++ b/test/spec/Feature/OpenApi/RootSpec.hs @@ -29,6 +29,7 @@ spec = [json| { "tableName": "orders_view", "tableSchema": "test", "tableDeletable": true, "tableUpdatable": true, - "tableInsertable": true, "tableDescription": null + "tableIsView":true, "tableInsertable": true, + "tableDescription": null } |] { matchHeaders = [matchContentTypeJson] } diff --git a/test/spec/Feature/Query/DeleteSpec.hs b/test/spec/Feature/Query/DeleteSpec.hs index f4a338785..eb211fd4d 100644 --- a/test/spec/Feature/Query/DeleteSpec.hs +++ b/test/spec/Feature/Query/DeleteSpec.hs @@ -3,7 +3,7 @@ module Feature.Query.DeleteSpec where import Network.Wai (Application) import Network.HTTP.Types -import Test.Hspec +import Test.Hspec hiding (pendingWith) import Test.Hspec.Wai import Test.Hspec.Wai.JSON @@ -213,7 +213,16 @@ spec = `shouldRespondWith` "" { matchStatus = 204 } + it "doesn't work with views" $ + request methodDelete "/limited_delete_items_view?limit=1&offset=1" + [("Prefer", "tx=commit")] + mempty + `shouldRespondWith` + [json| {"hint":null,"details":null,"code":"PGRST507","message":"limit/offset is not implemented for views"} |] + { matchStatus = 501 } + it "works with views with an inferred pk" $ do + pendingWith "not implemented yet" get "/limited_delete_items_view" `shouldRespondWith` [json|[ diff --git a/test/spec/Feature/Query/UpdateSpec.hs b/test/spec/Feature/Query/UpdateSpec.hs index 03599d0e2..7edc25436 100644 --- a/test/spec/Feature/Query/UpdateSpec.hs +++ b/test/spec/Feature/Query/UpdateSpec.hs @@ -520,7 +520,16 @@ spec = do `shouldRespondWith` "" { matchStatus = 204 } + it "doesn't work with views" $ + request methodPatch "/limited_update_items_view?limit=1&offset=1" + [("Prefer", "tx=commit")] + [json| {"name": "updated-item"} |] + `shouldRespondWith` + [json| {"hint":null,"details":null,"code":"PGRST507","message":"limit/offset is not implemented for views"} |] + { matchStatus = 501 } + it "works with views with an inferred pk" $ do + pendingWith "not implemented yet" get "/limited_update_items_view" `shouldRespondWith` [json|[