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

break: remove limited updated/delete feature (#3907)

Browse Source 
BREAKING CHANGE

As agreed on https://github.com/PostgREST/postgrest/issues/3013#issuecomment-1770186262,
this removes the limited update/delete feature.

The feature was complicated, largely unused and caused other bugs in
mutations.

It was added in #2195 and #2211.
This commit is contained in:
Steve Chavez
2025-02-12 14:48:08 -05:00
committed by GitHub
parent 94f0edb61a
commit 307692c325
19 changed files with 20 additions and 549 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/spec/Feature/Query/LimitedMutationSpec.hs
-305
View File
@@ -1,305 +0,0 @@
module Feature.Query.LimitedMutationSpec where
import Data.Aeson.QQ
import Network.Wai (Application)
import Network.HTTP.Types
import Test.Hspec hiding (pendingWith)
import Test.Hspec.Wai
import Test.Hspec.Wai.JSON
import Protolude hiding (get)
import SpecHelper
tblDataBefore = [aesonQQ|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "item-2" }
, { "id": 3, "name": "item-3" }
]|]
spec :: SpecWith ((), Application)
spec = do
describe "limited delete" $ do
it "works with the limit and offset query params" $
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items?order=id&limit=1&offset=1" mempty mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 3, "name": "item-3" }
]|]
it "works with the limit query param plus a filter" $
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items?order=id&limit=1&id=gt.1" mempty mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 3, "name": "item-3" }
]|]
it "fails without an explicit order by" $
request methodDelete "/limited_delete_items?limit=1&offset=1"
mempty
mempty
`shouldRespondWith`
[json| {
"code":"PGRST109",
"hint": "Apply an 'order' using unique column(s)",
"details": null,
"message": "A 'limit' was applied without an explicit 'order'"
}|]
{ matchStatus = 400 }
it "fails when not ordering by a unique column" $
request methodDelete "/limited_delete_items_wnonuniq_view?order=static&limit=1"
mempty
mempty
`shouldRespondWith`
[json| {
"code":"PGRST110",
"hint": null,
"details":"Results contain 3 rows changed but the maximum number allowed is 1",
"message":"The maximum number of rows allowed to change was surpassed"
}|]
{ matchStatus = 400 }
it "works with views with an explicit order by unique col" $
baseTable "limited_delete_items_view" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items_view?order=id&limit=1&offset=1" mempty mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 3, "name": "item-3" }
]|]
it "works with views with an explicit order by composite pk" $
baseTable "limited_delete_items_cpk_view" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items_cpk_view?order=id,name&limit=1&offset=1" mempty mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 3, "name": "item-3" }
]|]
it "works on a table without a pk by ordering by 'ctid'" $
baseTable "limited_delete_items_no_pk" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items_no_pk?order=ctid&limit=1&offset=1" mempty mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 3, "name": "item-3" }
]|]
it "ignores the Range header" $ do
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items"
(rangeHdrs (ByteRangeFromTo 0 0)) mempty
`shouldMutateInto`
[json|[]|]
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items?id=gte.2"
(rangeHdrs (ByteRangeFromTo 0 0)) mempty
`shouldMutateInto`
[json|[ { "id": 1, "name": "item-1" } ]|]
it "ignores the Range header and does not do a limited delete" $
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items?order=id"
(rangeHdrs (ByteRangeFromTo 0 0)) mempty
`shouldMutateInto`
[json|[]|]
it "ignores the Range header and does not throw an invalid range error" $
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items?order=id&limit=1&offset=1"
(rangeHdrs (ByteRangeFromTo 0 0)) mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 3, "name": "item-3" }
]|]
it "ignores the Range header but not the limit and offset params" $
baseTable "limited_delete_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodDelete "/limited_delete_items?order=id&limit=2&offset=1"
(rangeHdrs (ByteRangeFromTo 1 1)) mempty
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
]|]
describe "limited update" $ do
it "works with the limit query param" $
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?order=id&limit=2" mempty
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "updated-item" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "item-3" }
]|]
it "works with the limit query param plus a filter" $
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?order=id&limit=1&id=gt.2" mempty
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "item-2" }
, { "id": 3, "name": "updated-item" }
]|]
it "works with the limit and offset query params" $
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?order=id&limit=1&offset=1" mempty
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "item-3" }
]|]
it "fails without an explicit order by" $
request methodPatch "/limited_update_items?limit=1&offset=1"
mempty
[json| {"name": "updated-item"} |]
`shouldRespondWith`
[json| {
"code":"PGRST109",
"hint": "Apply an 'order' using unique column(s)",
"details": null,
"message": "A 'limit' was applied without an explicit 'order'"
}|]
{ matchStatus = 400 }
it "fails when not ordering by a unique column" $
request methodPatch "/limited_update_items_wnonuniq_view?order=static&limit=1"
mempty
[json| {"name": "updated-item"} |]
`shouldRespondWith`
[json| {
"code":"PGRST110",
"hint": null,
"details":"Results contain 3 rows changed but the maximum number allowed is 1",
"message":"The maximum number of rows allowed to change was surpassed"
}|]
{ matchStatus = 400 }
it "works with views with an explicit order by unique col" $
baseTable "limited_update_items_view" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items_view?order=id&limit=1&offset=1" mempty
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "item-3" }
]|]
it "works with views with an explicit order by composite pk" $
baseTable "limited_update_items_cpk_view" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items_cpk_view?order=id,name&limit=1&offset=1" mempty
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "item-3" }
]|]
it "works on a table without a pk by ordering by 'ctid'" $
baseTable "limited_update_items_no_pk" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items_no_pk?order=ctid&limit=1" mempty
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "updated-item" }
, { "id": 2, "name": "item-2" }
, { "id": 3, "name": "item-3" }
]|]
it "ignores the Range header" $ do
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items"
(rangeHdrs (ByteRangeFromTo 0 0))
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "updated-item" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "updated-item" }
]|]
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?id=gte.2"
(rangeHdrs (ByteRangeFromTo 0 0))
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "updated-item" }
]|]
it "ignores the Range header and does not do a limited update" $
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?order=id"
(rangeHdrs (ByteRangeFromTo 0 0))
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "updated-item" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "updated-item" }
]|]
it "ignores the Range header and does not throw an invalid range error" $
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?order=id&limit=1&offset=1"
(rangeHdrs (ByteRangeFromTo 0 0))
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "item-3" }
]|]
it "ignores the Range header but not the limit and offset params" $
baseTable "limited_update_items" "id" tblDataBefore
`mutatesWith`
requestMutation methodPatch "/limited_update_items?order=id&limit=2&offset=1"
(rangeHdrs (ByteRangeFromTo 1 1))
[json| {"name": "updated-item"} |]
`shouldMutateInto`
[json|[
{ "id": 1, "name": "item-1" }
, { "id": 2, "name": "updated-item" }
, { "id": 3, "name": "updated-item" }
]|]
test/spec/Feature/Query/UpdateSpec.hs
-12
View File
@@ -361,18 +361,6 @@ spec = do
, matchHeaders = ["Preference-Applied" <:> "missing=default, return=representation"]
}
it "updates with limit/offset using table default values(field-with_sep) when json keys are undefined" $ do
request methodPatch "/complex_items?select=id,name&columns=name,field-with_sep&limit=1&offset=2&order=id"
[("Prefer", "return=representation"), ("Prefer", "missing=default")]
[json|{"name": "Tres"}|]
`shouldRespondWith`
[json|[
{"id":3,"name":"Tres"}
]|]
{ matchStatus = 200
, matchHeaders = ["Preference-Applied" <:> "missing=default, return=representation"]
}
it "updates table default values(field-with_sep) when json keys are undefined" $ do
request methodPatch "/complex_items?id=eq.3&columns=name,field-with_sep"
[("Prefer", "return=representation"), ("Prefer", "missing=default")]
test/spec/Main.hs
-4
View File
@@ -47,7 +47,6 @@ import qualified Feature.Query.EmbedInnerJoinSpec
import qualified Feature.Query.ErrorSpec
import qualified Feature.Query.InsertSpec
import qualified Feature.Query.JsonOperatorSpec
import qualified Feature.Query.LimitedMutationSpec
import qualified Feature.Query.MultipleSchemaSpec
import qualified Feature.Query.NullsStripSpec
import qualified Feature.Query.PgSafeUpdateSpec
@@ -269,9 +268,6 @@ main = do
before forceRollbackApp $
describe "Feature.RollbackForcedSpec" Feature.RollbackSpec.forced
before withApp $
describe "Feature.Query.LimitedMutationSpec" Feature.Query.LimitedMutationSpec.spec
-- This test runs with a pre request to enable the pg-safeupdate library per-session.
-- This needs to run last, because once pg safe update is loaded, it can't be unloaded again.
before pgSafeUpdateApp $
test/spec/SpecHelper.hs
-33
View File
@@ -24,7 +24,6 @@ import Text.Regex.TDFA ((=~))
import Network.HTTP.Types
import Test.Hspec
import Test.Hspec.Wai
import Test.Hspec.Wai.JSON
import Text.Heredoc
import Data.String (String)
@@ -299,38 +298,6 @@ isErrorFormat s =
keys = maybe S.empty M.keysSet obj
validKeys = S.fromList ["message", "details", "hint", "code"]
-- | Follows these steps to verify if the table data changed in the db:
-- * Verifies the table data in the db before the change
-- * Does the mutation
-- * Verifies that the table data changed in the db
-- * Resets the table with the original data
shouldMutateInto :: MutationCheck -> ResponseMatcher -> WaiExpectation ()
shouldMutateInto (MutationCheck (BaseTable tblName tblOrd dataBefore) mutation) dataAfter = do
get ("/" <> tblName) `shouldRespondWith` [json|#{dataBefore}|]
mutation
get ("/" <> tblName <> "?order=" <> tblOrd) `shouldRespondWith` dataAfter
request methodPost "/rpc/reset_table"
[("Prefer", "tx=commit")]
[json| {"tbl_name": #{decodeUtf8 tblName}, "tbl_data": #{dataBefore}} |]
`shouldRespondWith` 204
-- | How the base table data will change using the requested mutation
mutatesWith :: BaseTable -> WaiExpectation () -> MutationCheck
mutatesWith = MutationCheck
-- | The original table data before it is modified.
-- The column order is needed for an accurate comparison after the mutation
baseTable :: ByteString -> ByteString -> JSON.Value -> BaseTable
baseTable = BaseTable
-- | The mutation (update/delete) that will be applied to the base table
requestMutation :: Method -> ByteString -> [Header] -> BL.ByteString -> WaiExpectation ()
requestMutation method path headers body =
request method path (("Prefer", "tx=commit") : headers) body `shouldRespondWith` "" { matchStatus = 204 }
data BaseTable = BaseTable ByteString ByteString JSON.Value
data MutationCheck = MutationCheck BaseTable (WaiExpectation ())
planCost :: SResponse -> Float
planCost resp =
let res = simpleBody resp ^? nth 0 . key "Plan" . key "Total Cost" in
test/spec/fixtures/data.sql
Vendored
-18
View File
@@ -741,24 +741,6 @@ INSERT INTO test.fav_numbers VALUES (ROW(0.5, 0.5), 'A'), (ROW(0.6, 0.6), 'B');
TRUNCATE TABLE test.arrays CASCADE;
INSERT INTO test.arrays VALUES (0, '{1,2,3}', '{{1,2,3},{4,5,6},{7,8,9}}'), (1, '{11,12,13}', '{{11,12,13},{14,15,16},{17,18,19}}');
TRUNCATE TABLE test.limited_update_items CASCADE;
INSERT INTO test.limited_update_items VALUES (1, 'item-1'), (2, 'item-2'), (3, 'item-3');
TRUNCATE TABLE test.limited_update_items_cpk CASCADE;
INSERT INTO test.limited_update_items_cpk VALUES (1, 'item-1'), (2, 'item-2'), (3, 'item-3');
TRUNCATE TABLE test.limited_update_items_no_pk CASCADE;
INSERT INTO test.limited_update_items_no_pk VALUES (1, 'item-1'), (2, 'item-2'), (3, 'item-3');
TRUNCATE TABLE test.limited_delete_items CASCADE;
INSERT INTO test.limited_delete_items VALUES (1, 'item-1'), (2, 'item-2'), (3, 'item-3');
TRUNCATE TABLE test.limited_delete_items_cpk CASCADE;
INSERT INTO test.limited_delete_items_cpk VALUES (1, 'item-1'), (2, 'item-2'), (3, 'item-3');
TRUNCATE TABLE test.limited_delete_items_no_pk CASCADE;
INSERT INTO test.limited_delete_items_no_pk VALUES (1, 'item-1'), (2, 'item-2'), (3, 'item-3');
TRUNCATE TABLE test.xmltest CASCADE;
INSERT INTO test.xmltest VALUES
(1, '<myxml>foo</myxml>'),
test/spec/fixtures/schema.sql
Vendored
-50
View File
@@ -2512,56 +2512,6 @@ CREATE AGGREGATE test.unsupported_agg (*) (
STYPE = int
);
create table limited_update_items(
id int primary key
, name text
);
create table limited_update_items_cpk(
id int
, name text
, primary key (id, name)
);
create table limited_update_items_no_pk(
id int
, name text
);
create view limited_update_items_view as
select * from limited_update_items;
create view limited_update_items_wnonuniq_view as
select *, 'static'::text as static from limited_update_items;
create view limited_update_items_cpk_view as
select * from limited_update_items_cpk;
create table limited_delete_items(
id int primary key
, name text
);
create table limited_delete_items_cpk(
id int
, name text
, primary key (id, name)
);
create table limited_delete_items_no_pk(
id int
, name text
);
create view limited_delete_items_view as
select * from limited_delete_items;
create view limited_delete_items_wnonuniq_view as
select *, 'static'::text as static from limited_delete_items;
create view limited_delete_items_cpk_view as
select * from limited_delete_items_cpk;
create function reset_table(tbl_name text default '', tbl_data json default '[]') returns void as $_$ begin
execute format(
$$