fix: max-affected preference not failing for rpc with strict handling
This commit is contained in:
committed by
Laurence Isla
parent
ea2d3aeb72
commit
f54aef4795
@@ -5,6 +5,17 @@ This project adheres to [Semantic Versioning](http://semver.org/).
|
|||||||
|
|
||||||
## Unreleased
|
## Unreleased
|
||||||
|
|
||||||
|
### Added
|
||||||
|
|
||||||
|
- Improve the `PGRST106` error when the requested schema is invalid by @laurenceisla in #4089
|
||||||
|
+ It now shows the invalid schema in the `message` field.
|
||||||
|
+ The exposed schemas are now listed in the `hint` instead of the `message` field.
|
||||||
|
- Improve error details of `PGRST301` error by @taimoorzaeem in #4051
|
||||||
|
|
||||||
|
### Fixed
|
||||||
|
|
||||||
|
- Fix `max-affected` preference not failing with RPC when `handling=strict` by @taimoorzaeem in #4100
|
||||||
|
|
||||||
## [13.0.2] - 2025-06-02
|
## [13.0.2] - 2025-06-02
|
||||||
|
|
||||||
### Fixed
|
### Fixed
|
||||||
|
|||||||
@@ -292,3 +292,7 @@ With :ref:`RPC <functions>`, the preference is honored completely on the basis o
|
|||||||
"details": "The query affects 14 rows",
|
"details": "The query affects 14 rows",
|
||||||
"hint": null
|
"hint": null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
|
||||||
|
It is important for functions to return ``SETOF`` or ``TABLE`` when called with ``max-affected`` preference. A violation of this would cause a :ref:`PGRST128 <pgrst128>` error.
|
||||||
|
|||||||
@@ -267,6 +267,10 @@ Related to the HTTP request elements.
|
|||||||
| | | implemented. |
|
| | | implemented. |
|
||||||
| PGRST127 | | |
|
| PGRST127 | | |
|
||||||
+---------------+-------------+-------------------------------------------------------------+
|
+---------------+-------------+-------------------------------------------------------------+
|
||||||
|
| .. _pgrst128: | 400 | ``max-affected`` preference is violated with ``RPC`` call. |
|
||||||
|
| | | See :ref:`prefer_max_affected`. |
|
||||||
|
| PGRST128 | | |
|
||||||
|
+---------------+-------------+-------------------------------------------------------------+
|
||||||
|
|
||||||
|
|
||||||
.. _pgrst2**:
|
.. _pgrst2**:
|
||||||
|
|||||||
@@ -96,6 +96,7 @@ data ApiRequestError
|
|||||||
| MaxAffectedViolationError Integer
|
| MaxAffectedViolationError Integer
|
||||||
| InvalidResourcePath
|
| InvalidResourcePath
|
||||||
| OpenAPIDisabled
|
| OpenAPIDisabled
|
||||||
|
| MaxAffectedRpcViolation
|
||||||
deriving Show
|
deriving Show
|
||||||
|
|
||||||
data QPError = QPError Text Text
|
data QPError = QPError Text Text
|
||||||
@@ -138,6 +139,7 @@ instance PgrstError ApiRequestError where
|
|||||||
status MaxAffectedViolationError{} = HTTP.status400
|
status MaxAffectedViolationError{} = HTTP.status400
|
||||||
status InvalidResourcePath = HTTP.status404
|
status InvalidResourcePath = HTTP.status404
|
||||||
status OpenAPIDisabled = HTTP.status404
|
status OpenAPIDisabled = HTTP.status404
|
||||||
|
status MaxAffectedRpcViolation = HTTP.status400
|
||||||
|
|
||||||
headers _ = mempty
|
headers _ = mempty
|
||||||
|
|
||||||
@@ -184,6 +186,7 @@ instance ErrorBody ApiRequestError where
|
|||||||
code InvalidResourcePath = "PGRST125"
|
code InvalidResourcePath = "PGRST125"
|
||||||
code OpenAPIDisabled = "PGRST126"
|
code OpenAPIDisabled = "PGRST126"
|
||||||
code NotImplemented{} = "PGRST127"
|
code NotImplemented{} = "PGRST127"
|
||||||
|
code MaxAffectedRpcViolation = "PGRST128"
|
||||||
|
|
||||||
-- MESSAGE: Text
|
-- MESSAGE: Text
|
||||||
message (QueryParamError (QPError msg _)) = msg
|
message (QueryParamError (QPError msg _)) = msg
|
||||||
@@ -209,6 +212,7 @@ instance ErrorBody ApiRequestError where
|
|||||||
message InvalidResourcePath = "Invalid path specified in request URL"
|
message InvalidResourcePath = "Invalid path specified in request URL"
|
||||||
message OpenAPIDisabled = "Root endpoint metadata is disabled"
|
message OpenAPIDisabled = "Root endpoint metadata is disabled"
|
||||||
message (NotImplemented _) = "Feature not implemented"
|
message (NotImplemented _) = "Feature not implemented"
|
||||||
|
message MaxAffectedRpcViolation = "Function must return SETOF or TABLE when max-affected preference is used with handling=strict"
|
||||||
|
|
||||||
-- DETAILS: Maybe JSON.Value
|
-- DETAILS: Maybe JSON.Value
|
||||||
details (QueryParamError (QPError _ dets)) = Just $ JSON.String dets
|
details (QueryParamError (QPError _ dets)) = Just $ JSON.String dets
|
||||||
|
|||||||
@@ -72,7 +72,8 @@ import PostgREST.SchemaCache.Routine (MediaHandler (..),
|
|||||||
RoutineParam (..),
|
RoutineParam (..),
|
||||||
funcReturnsCompositeAlias,
|
funcReturnsCompositeAlias,
|
||||||
funcReturnsScalar,
|
funcReturnsScalar,
|
||||||
funcReturnsSetOfScalar)
|
funcReturnsSetOfScalar,
|
||||||
|
funcReturnsSingle)
|
||||||
import PostgREST.SchemaCache.Table (Column (..), Table (..),
|
import PostgREST.SchemaCache.Table (Column (..), Table (..),
|
||||||
TablesMap,
|
TablesMap,
|
||||||
tableColumnsList,
|
tableColumnsList,
|
||||||
@@ -172,7 +173,7 @@ mutateReadPlan mutation apiRequest@ApiRequest{iPreferences=Preferences{..},..}
|
|||||||
return $ MutateReadPlan rPlan mPlan SQL.Write handler mediaType mutation qi
|
return $ MutateReadPlan rPlan mPlan SQL.Write handler mediaType mutation qi
|
||||||
|
|
||||||
callReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> InvokeMethod -> Either Error CallReadPlan
|
callReadPlan :: QualifiedIdentifier -> AppConfig -> SchemaCache -> ApiRequest -> InvokeMethod -> Either Error CallReadPlan
|
||||||
callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{preferHandling, invalidPrefs},..} invMethod = do
|
callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferences{preferHandling, invalidPrefs, preferMaxAffected},..} invMethod = do
|
||||||
let paramKeys = case invMethod of
|
let paramKeys = case invMethod of
|
||||||
InvRead _ -> S.fromList $ fst <$> qsParams'
|
InvRead _ -> S.fromList $ fst <$> qsParams'
|
||||||
Inv -> iColumns
|
Inv -> iColumns
|
||||||
@@ -192,10 +193,15 @@ callReadPlan identifier conf sCache apiRequest@ApiRequest{iPreferences=Preferenc
|
|||||||
cPlan = callPlan proc apiRequest paramKeys args rPlan
|
cPlan = callPlan proc apiRequest paramKeys args rPlan
|
||||||
(handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest relIdentifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
|
(handler, mediaType) <- mapLeft ApiRequestError $ negotiateContent conf apiRequest relIdentifier iAcceptMediaType (dbMediaHandlers sCache) (hasDefaultSelect rPlan)
|
||||||
if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
|
if not (null invalidPrefs) && preferHandling == Just Strict then Left $ ApiRequestError $ InvalidPreferences invalidPrefs else Right ()
|
||||||
|
failMaxAffectedRpcReturnsSingle (preferMaxAffected, preferHandling) proc
|
||||||
return $ CallReadPlan rPlan cPlan txMode proc handler mediaType invMethod identifier
|
return $ CallReadPlan rPlan cPlan txMode proc handler mediaType invMethod identifier
|
||||||
where
|
where
|
||||||
qsParams' = QueryParams.qsParams iQueryParams
|
qsParams' = QueryParams.qsParams iQueryParams
|
||||||
|
|
||||||
|
failMaxAffectedRpcReturnsSingle :: (Maybe PreferMaxAffected, Maybe PreferHandling) -> Routine -> Either Error ()
|
||||||
|
failMaxAffectedRpcReturnsSingle (Just (PreferMaxAffected _), Just Strict) rout = if funcReturnsSingle rout then Left $ ApiRequestError MaxAffectedRpcViolation else Right ()
|
||||||
|
failMaxAffectedRpcReturnsSingle _ _ = Right ()
|
||||||
|
|
||||||
hasDefaultSelect :: ReadPlanTree -> Bool
|
hasDefaultSelect :: ReadPlanTree -> Bool
|
||||||
hasDefaultSelect (Node ReadPlan{select=[CoercibleSelectField{csField=CoercibleField{cfName}}]} []) = cfName == "*"
|
hasDefaultSelect (Node ReadPlan{select=[CoercibleSelectField{csField=CoercibleField{cfName}}]} []) = cfName == "*"
|
||||||
hasDefaultSelect _ = False
|
hasDefaultSelect _ = False
|
||||||
|
|||||||
@@ -192,16 +192,25 @@ spec =
|
|||||||
{ matchStatus = 204
|
{ matchStatus = 204
|
||||||
, matchHeaders = ["Preference-Applied" <:> "handling=lenient"]}
|
, matchHeaders = ["Preference-Applied" <:> "handling=lenient"]}
|
||||||
|
|
||||||
it "should fail with rpc when deleting rows more than prefered" $
|
context "test Prefer: max-affected with rpc" $ do
|
||||||
request methodPost "/rpc/delete_all_items"
|
it "should fail with rpc when deleting rows more than prefered with returns setof" $
|
||||||
|
request methodPost "/rpc/delete_items_returns_setof"
|
||||||
[("Prefer", "handling=strict, max-affected=10")]
|
[("Prefer", "handling=strict, max-affected=10")]
|
||||||
""
|
""
|
||||||
`shouldRespondWith`
|
`shouldRespondWith`
|
||||||
[json| {"code":"PGRST124","details":"The query affects 15 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"} |]
|
[json| {"code":"PGRST124","details":"The query affects 15 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"} |]
|
||||||
{ matchStatus = 400 }
|
{ matchStatus = 400 }
|
||||||
|
|
||||||
it "should succeed with rpc deleting rows less than prefered" $
|
it "should fail with rpc when deleting rows more than prefered with returns table" $
|
||||||
request methodPost "/rpc/delete_all_items"
|
request methodPost "/rpc/delete_items_returns_table"
|
||||||
|
[("Prefer", "handling=strict, max-affected=10")]
|
||||||
|
""
|
||||||
|
`shouldRespondWith`
|
||||||
|
[json| {"code":"PGRST124","details":"The query affects 15 rows","hint":null,"message":"Query result exceeds max-affected preference constraint"} |]
|
||||||
|
{ matchStatus = 400 }
|
||||||
|
|
||||||
|
it "should succeed with rpc deleting rows less than prefered with returns setof" $
|
||||||
|
request methodPost "/rpc/delete_items_returns_setof"
|
||||||
[("Prefer", "handling=strict, max-affected=20")]
|
[("Prefer", "handling=strict, max-affected=20")]
|
||||||
""
|
""
|
||||||
`shouldRespondWith`
|
`shouldRespondWith`
|
||||||
@@ -209,3 +218,21 @@ spec =
|
|||||||
{"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},
|
{"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},
|
||||||
{"id":14},{"id":15}]|]
|
{"id":14},{"id":15}]|]
|
||||||
{ matchStatus = 200 }
|
{ matchStatus = 200 }
|
||||||
|
|
||||||
|
it "should succeed with rpc deleting rows less than prefered with returns table" $
|
||||||
|
request methodPost "/rpc/delete_items_returns_table"
|
||||||
|
[("Prefer", "handling=strict, max-affected=20")]
|
||||||
|
""
|
||||||
|
`shouldRespondWith`
|
||||||
|
[json|[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":6},{"id":7},
|
||||||
|
{"id":8},{"id":9},{"id":10},{"id":11},{"id":12},{"id":13},
|
||||||
|
{"id":14},{"id":15}]|]
|
||||||
|
{ matchStatus = 200 }
|
||||||
|
|
||||||
|
it "should fail with rpc when returns void with handling=strict" $
|
||||||
|
request methodPost "/rpc/delete_items_returns_void"
|
||||||
|
[("Prefer", "handling=strict, max-affected=20")]
|
||||||
|
""
|
||||||
|
`shouldRespondWith`
|
||||||
|
[json| {"code":"PGRST128","details":null,"hint":null,"message":"Function must return SETOF or TABLE when max-affected preference is used with handling=strict"} |]
|
||||||
|
{ matchStatus = 400 }
|
||||||
|
|||||||
Vendored
+9
-1
@@ -3817,6 +3817,14 @@ where oid = 'test.collision_test_func'::regproc::oid;
|
|||||||
comment on function test.collision_test_func(id integer) is 'fizzbuzz';
|
comment on function test.collision_test_func(id integer) is 'fizzbuzz';
|
||||||
|
|
||||||
|
|
||||||
create or replace function test.delete_all_items() returns setof items as $$
|
create or replace function test.delete_items_returns_setof() returns setof items as $$
|
||||||
delete from items where id <= 15 returning *; -- deletes 15 items, then return them
|
delete from items where id <= 15 returning *; -- deletes 15 items, then return them
|
||||||
$$ language sql;
|
$$ language sql;
|
||||||
|
|
||||||
|
create or replace function test.delete_items_returns_table() returns table(id bigint) as $$
|
||||||
|
delete from items where id <= 15 returning *;
|
||||||
|
$$ language sql;
|
||||||
|
|
||||||
|
create or replace function test.delete_items_returns_void() returns void as $$
|
||||||
|
delete from items;
|
||||||
|
$$ language sql;
|
||||||
|
|||||||
Reference in New Issue
Block a user