From cde8eb819e51e74796f27e6d45f8b78d383d2045 Mon Sep 17 00:00:00 2001 From: Taimoor Zaeem Date: Mon, 10 Jun 2024 11:33:31 +0500 Subject: [PATCH] fix: mixing offset and limit with Range header --- src/PostgREST/ApiRequest.hs | 39 ++---- src/PostgREST/ApiRequest/QueryParams.hs | 129 +++++++++--------- src/PostgREST/ApiRequest/Types.hs | 3 +- src/PostgREST/Error.hs | 16 ++- src/PostgREST/Plan.hs | 55 +++++--- src/PostgREST/Plan/MutatePlan.hs | 15 +- src/PostgREST/Plan/ReadPlan.hs | 2 + src/PostgREST/Query.hs | 11 +- src/PostgREST/Query/QueryBuilder.hs | 29 ++-- src/PostgREST/Query/SqlFragment.hs | 14 +- src/PostgREST/RangeQuery.hs | 2 +- src/PostgREST/Response.hs | 8 +- test/spec/Feature/Query/QueryLimitedSpec.hs | 4 +- test/spec/Feature/Query/QuerySpec.hs | 5 +- test/spec/Feature/Query/RangeSpec.hs | 77 +++-------- test/spec/Feature/Query/RelatedQueriesSpec.hs | 8 +- test/spec/Feature/Query/RpcSpec.hs | 2 +- 17 files changed, 204 insertions(+), 215 deletions(-) diff --git a/src/PostgREST/ApiRequest.hs b/src/PostgREST/ApiRequest.hs index 5b0c97cee1..249f7476f4 100644 --- a/src/PostgREST/ApiRequest.hs +++ b/src/PostgREST/ApiRequest.hs @@ -2,8 +2,9 @@ Module : PostgREST.Request.ApiRequest Description : PostgREST functions to translate HTTP request to a domain type called ApiRequest. -} -{-# LANGUAGE LambdaCase #-} -{-# LANGUAGE NamedFieldPuns #-} +{-# LANGUAGE LambdaCase #-} +{-# LANGUAGE NamedFieldPuns #-} +{-# LANGUAGE RecordWildCards #-} -- TODO: This module shouldn't depend on SchemaCache module PostgREST.ApiRequest ( ApiRequest(..) @@ -35,8 +36,7 @@ import Data.Either.Combinators (mapBoth) import Control.Arrow ((***)) import Data.Aeson.Types (emptyArray, emptyObject) import Data.List (lookup) -import Data.Ranged.Ranges (emptyRange, rangeIntersection, - rangeIsEmpty) +import Data.Ranged.Ranges (rangeIsEmpty) import Network.HTTP.Types.Header (RequestHeaders, hCookie) import Network.HTTP.Types.URI (parseSimpleQuery) import Network.Wai (Request (..)) @@ -50,8 +50,6 @@ import PostgREST.Config (AppConfig (..), OpenAPIMode (..)) import PostgREST.MediaType (MediaType (..)) import PostgREST.RangeQuery (NonnegRange, allRange, - convertToLimitZeroRange, - hasLimitZero, rangeRequested) import PostgREST.SchemaCache (SchemaCache (..)) import PostgREST.SchemaCache.Identifiers (FieldName, @@ -111,8 +109,7 @@ data Action -} data ApiRequest = ApiRequest { iAction :: Action -- ^ Action on the resource - , iRange :: HM.HashMap Text NonnegRange -- ^ Requested range of rows within response - , iTopLevelRange :: NonnegRange -- ^ Requested range of rows from the top level + , iRange :: NonnegRange -- ^ Requested range of rows from the selected resource , iPayload :: Maybe Payload -- ^ Data sent by client and used for mutation actions , iPreferences :: Preferences.Preferences -- ^ Prefer header values , iQueryParams :: QueryParams.QueryParams @@ -134,12 +131,11 @@ userApiRequest conf req reqBody sCache = do (schema, negotiatedByProfile) <- getSchema conf hdrs method act <- getAction resource schema method qPrms <- first QueryParamError $ QueryParams.parse (actIsInvokeSafe act) $ rawQueryString req - (topLevelRange, ranges) <- getRanges method qPrms hdrs + hRange <- getRange method qPrms hdrs (payload, columns) <- getPayload reqBody contentMediaType qPrms act return $ ApiRequest { iAction = act - , iRange = ranges - , iTopLevelRange = topLevelRange + , iRange = hRange , iPayload = payload , iPreferences = Preferences.fromHeaders (configDbTxAllowOverride conf) (dbTimezones sCache) hdrs , iQueryParams = qPrms @@ -217,24 +213,17 @@ getSchema AppConfig{configDbSchemas} hdrs method = do acceptProfile = T.decodeUtf8 <$> lookupHeader "Accept-Profile" lookupHeader = flip lookup hdrs -getRanges :: ByteString -> QueryParams -> RequestHeaders -> Either ApiRequestError (NonnegRange, HM.HashMap Text NonnegRange) -getRanges method QueryParams{qsOrder,qsRanges} hdrs - | isInvalidRange = Left $ InvalidRange (if rangeIsEmpty headerRange then LowerGTUpper else NegativeLimit) - | method `elem` ["PATCH", "DELETE"] && not (null qsRanges) && null qsOrder = Left LimitNoOrderError - | method == "PUT" && topLevelRange /= allRange = Left PutLimitNotAllowedError - | otherwise = Right (topLevelRange, ranges) +getRange :: ByteString -> QueryParams -> RequestHeaders -> Either ApiRequestError NonnegRange +getRange method QueryParams{..} hdrs + | rangeIsEmpty headerRange = Left $ InvalidRange LowerGTUpper -- A Range is empty unless its upper boundary is GT its lower boundary + | method `elem` ["PATCH","DELETE"] && not (null qsLimit) && null qsOrder = Left LimitNoOrderError + | method == "PUT" && offsetLimitPresent = Left PutLimitNotAllowedError + | otherwise = Right headerRange where -- According to the RFC (https://www.rfc-editor.org/rfc/rfc9110.html#name-range), -- the Range header must be ignored for all methods other than GET headerRange = if method == "GET" then rangeRequested hdrs else allRange - limitRange = fromMaybe allRange (HM.lookup "limit" qsRanges) - headerAndLimitRange = rangeIntersection headerRange limitRange - -- Bypass all the ranges and send only the limit zero range (0 <= x <= -1) if - -- limit=0 is present in the query params (not allowed for the Range header) - ranges = HM.insert "limit" (convertToLimitZeroRange limitRange headerAndLimitRange) qsRanges - -- The only emptyRange allowed is the limit zero range - isInvalidRange = topLevelRange == emptyRange && not (hasLimitZero limitRange) - topLevelRange = fromMaybe allRange $ HM.lookup "limit" ranges -- if no limit is specified, get all the request rows + offsetLimitPresent = not (null qsOffset && null qsLimit) getPayload :: RequestBody -> MediaType -> QueryParams.QueryParams -> Action -> Either ApiRequestError (Maybe Payload, S.Set FieldName) getPayload reqBody contentMediaType QueryParams{qsColumns} action = do diff --git a/src/PostgREST/ApiRequest/QueryParams.hs b/src/PostgREST/ApiRequest/QueryParams.hs index f9150e04e7..3c83d554a1 100644 --- a/src/PostgREST/ApiRequest/QueryParams.hs +++ b/src/PostgREST/ApiRequest/QueryParams.hs @@ -9,11 +9,10 @@ module PostgREST.ApiRequest.QueryParams ( parse , QueryParams(..) - , pRequestRange + , pTreePath ) where import qualified Data.ByteString.Char8 as BS -import qualified Data.HashMap.Strict as HM import qualified Data.List as L import qualified Data.Set as S import qualified Data.Text as T @@ -22,42 +21,42 @@ import qualified Network.HTTP.Base as HTTP import qualified Network.HTTP.Types.URI as HTTP import qualified Text.ParserCombinators.Parsec as P -import Control.Arrow ((***)) -import Data.Either.Combinators (mapLeft) -import Data.List (init, last) -import Data.Ranged.Boundaries (Boundary (..)) -import Data.Ranged.Ranges (Range (..)) -import Data.Tree (Tree (..)) -import Text.Parsec.Error (errorMessages, - showErrorMessages) -import Text.ParserCombinators.Parsec (GenParser, ParseError, Parser, - anyChar, between, char, choice, - digit, eof, errorPos, letter, - lookAhead, many1, noneOf, - notFollowedBy, oneOf, - optionMaybe, sepBy, sepBy1, - string, try, ()) - -import PostgREST.RangeQuery (NonnegRange, allRange, - rangeGeq, rangeLimit, - rangeOffset, restrictRange) +import Control.Arrow ((***)) +import Data.Either.Combinators (mapLeft) +import Data.List (init, last) +import Data.Tree (Tree (..)) +import PostgREST.ApiRequest.Types (AggregateFunction (..), + EmbedParam (..), EmbedPath, + Field, Filter (..), + FtsOperator (..), Hint, + JoinType (..), + JsonOperand (..), + JsonOperation (..), + JsonPath, ListVal, + LogicOperator (..), + LogicTree (..), OpExpr (..), + OpQuantifier (..), + Operation (..), + OrderDirection (..), + OrderNulls (..), + OrderTerm (..), + QPError (..), + QuantOperator (..), + SelectItem (..), + SimpleOperator (..), + SingleVal, TrileanVal (..)) import PostgREST.SchemaCache.Identifiers (FieldName) - -import PostgREST.ApiRequest.Types (AggregateFunction (..), - EmbedParam (..), EmbedPath, Field, - Filter (..), FtsOperator (..), - Hint, JoinType (..), - JsonOperand (..), - JsonOperation (..), JsonPath, - ListVal, LogicOperator (..), - LogicTree (..), OpExpr (..), - OpQuantifier (..), Operation (..), - OrderDirection (..), - OrderNulls (..), OrderTerm (..), - QPError (..), QuantOperator (..), - SelectItem (..), - SimpleOperator (..), SingleVal, - TrileanVal (..)) +import Text.Parsec.Error (errorMessages, + showErrorMessages) +import Text.ParserCombinators.Parsec (GenParser, ParseError, + Parser, anyChar, between, + char, choice, digit, eof, + errorPos, letter, lookAhead, + many1, noneOf, + notFollowedBy, oneOf, + optionMaybe, sepBy, sepBy1, + string, try, ()) +import Text.Read (read) import Protolude hiding (Sum, try) @@ -67,8 +66,10 @@ data QueryParams = -- ^ Canonical representation of the query params, sorted alphabetically , qsParams :: [(Text, Text)] -- ^ Parameters for RPC calls - , qsRanges :: HM.HashMap Text (Range Integer) - -- ^ Ranges derived from &limit and &offset params + , qsOffset :: [(EmbedPath, Integer)] + -- ^ &offset parameter + , qsLimit :: [(EmbedPath, Integer)] + -- ^ &limit parameter , qsOrder :: [(EmbedPath, [OrderTerm])] -- ^ &order parameters for each level , qsLogic :: [(EmbedPath, LogicTree)] @@ -115,6 +116,8 @@ parse :: Bool -> ByteString -> Either QPError QueryParams parse isRpcRead qs = do rOrd <- pRequestOrder `traverse` order rLogic <- pRequestLogicTree `traverse` logic + rOffset <- pRequestOffset `traverse` offset + rLimit <- pRequestLimit `traverse` limit rCols <- pRequestColumns columns rSel <- pRequestSelect select (rFlts, params) <- L.partition hasOp <$> pRequestFilter isRpcRead `traverse` filters @@ -125,7 +128,7 @@ parse isRpcRead qs = do params' = mapMaybe (\case {(_, Filter (fld, _) (NoOpExpr v)) -> Just (fld,v); _ -> Nothing}) params rFltsRoot' = snd <$> rFltsRoot - return $ QueryParams canonical params' ranges rOrd rLogic rCols rSel rFlts rFltsRoot' rFltsNotRoot rFltsFields rOnConflict + return $ QueryParams canonical params' rOffset rLimit rOrd rLogic rCols rSel rFlts rFltsRoot' rFltsNotRoot rFltsFields rOnConflict where hasRootFilter, hasOp :: (EmbedPath, Filter) -> Bool hasRootFilter ([], _) = True @@ -138,9 +141,8 @@ parse isRpcRead qs = do onConflict = lookupParam "on_conflict" columns = lookupParam "columns" order = filter (endingIn ["order"] . fst) nonemptyParams - limits = filter (endingIn ["limit"] . fst) nonemptyParams - -- Replace .offset ending with .limit to be able to match those params later in a map - offsets = first (replaceLast "limit") <$> filter (endingIn ["offset"] . fst) nonemptyParams + offset = filter (endingIn ["offset"] . fst) nonemptyParams + limit = filter (endingIn ["limit"] . fst) nonemptyParams lookupParam :: Text -> Maybe Text lookupParam needle = toS <$> join (L.lookup needle qParams) nonemptyParams = mapMaybe (\(k, v) -> (k,) <$> v) qParams @@ -155,7 +157,7 @@ parse isRpcRead qs = do . map (join (***) BS.unpack . second (fromMaybe mempty)) $ qString - endingIn:: [Text] -> Text -> Bool + endingIn :: [Text] -> Text -> Bool endingIn xx key = lastWord `elem` xx where lastWord = L.last $ T.split (== '.') key @@ -164,21 +166,6 @@ parse isRpcRead qs = do reserved = ["select", "columns", "on_conflict"] reservedEmbeddable = ["order", "limit", "offset", "and", "or"] - replaceLast x s = T.intercalate "." $ L.init (T.split (=='.') s) <> [x] - - ranges :: HM.HashMap Text (Range Integer) - ranges = HM.unionWith f limitParams offsetParams - where - f rl ro = Range (BoundaryBelow o) (BoundaryAbove $ o + l - 1) - where - l = fromMaybe 0 $ rangeLimit rl - o = rangeOffset ro - - limitParams = - HM.fromList [(k, restrictRange (readMaybe v) allRange) | (k,v) <- limits] - - offsetParams = - HM.fromList [(k, maybe allRange rangeGeq (readMaybe v)) | (k,v) <- offsets] simpleOperator :: Parser SimpleOperator simpleOperator = @@ -243,11 +230,19 @@ pRequestOrder (k, v) = mapError $ (,) <$> path <*> ord' path = fst <$> treePath ord' = P.parse pOrder ("failed to parse order (" ++ toS v ++ ")") $ toS v -pRequestRange :: (Text, NonnegRange) -> Either QPError (EmbedPath, NonnegRange) -pRequestRange (k, v) = mapError $ (,) <$> path <*> pure v +pRequestOffset :: (Text, Text) -> Either QPError (EmbedPath, Integer) +pRequestOffset (k,v) = mapError $ (,) <$> path <*> int where treePath = P.parse pTreePath ("failed to parse tree path (" ++ toS k ++ ")") $ toS k path = fst <$> treePath + int = P.parse pInt ("failed to parse offset parameter (" <> toS v <> ")") $ toS v + +pRequestLimit :: (Text, Text) -> Either QPError (EmbedPath, Integer) +pRequestLimit (k,v) = mapError $ (,) <$> path <*> int + where + treePath = P.parse pTreePath ("failed to parse tree path (" ++ toS k ++ ")") $ toS k + path = fst <$> treePath + int = P.parse pInt ("failed to parse limit parameter (" <> toS v <> ")") $ toS v pRequestLogicTree :: (Text, Text) -> Either QPError (EmbedPath, LogicTree) pRequestLogicTree (k, v) = mapError $ (,) <$> embedPath <*> logicTree @@ -842,6 +837,18 @@ pLogicPath = do notOp = "not." <> op return (filter (/= "not") (init path), if "not" `elem` path then notOp else op) +pInt :: Parser Integer +pInt = pPosInt <|> pNegInt + where + pPosInt :: Parser Integer + pPosInt = many1 digit <&> read + + pNegInt :: Parser Integer + pNegInt = do + _ <- char '-' + n <- many1 digit + return ((-1) * read n) + pColumns :: Parser [FieldName] pColumns = pFieldName `sepBy1` lexeme (char ',') diff --git a/src/PostgREST/ApiRequest/Types.hs b/src/PostgREST/ApiRequest/Types.hs index e4fb6dc323..6194d8dfc5 100644 --- a/src/PostgREST/ApiRequest/Types.hs +++ b/src/PostgREST/ApiRequest/Types.hs @@ -108,8 +108,7 @@ data RaiseError | NoDetail deriving Show data RangeError - = NegativeLimit - | LowerGTUpper + = LowerGTUpper | OutOfBounds Text Text deriving Show diff --git a/src/PostgREST/Error.hs b/src/PostgREST/Error.hs index 9c7d6d3a6f..29f1761855 100644 --- a/src/PostgREST/Error.hs +++ b/src/PostgREST/Error.hs @@ -84,7 +84,7 @@ instance PgrstError ApiRequestError where status UnacceptableFilter{} = HTTP.status400 status UnacceptableSchema{} = HTTP.status406 status UnsupportedMethod{} = HTTP.status405 - status LimitNoOrderError = HTTP.status400 + status LimitNoOrderError{} = HTTP.status400 status ColumnNotFound{} = HTTP.status400 status GucHeadersError = HTTP.status500 status GucStatusError = HTTP.status500 @@ -118,7 +118,6 @@ instance JSON.ToJSON ApiRequestError where ApiRequestErrorCode03 "Requested range not satisfiable" (Just $ case rangeError of - NegativeLimit -> "Limit should be greater than or equal to zero." LowerGTUpper -> "The lower boundary must be lower than or equal to the upper boundary in the Range header." OutOfBounds lower total -> JSON.String $ "An offset of " <> lower <> " was requested, but there are only " <> total <> " rows.") Nothing @@ -141,7 +140,10 @@ instance JSON.ToJSON ApiRequestError where (Just $ JSON.String $ "Verify that '" <> resource <> "' is included in the 'select' query parameter.") toJSON LimitNoOrderError = toJsonPgrstError - ApiRequestErrorCode09 "A 'limit' was applied without an explicit 'order'" Nothing (Just "Apply an 'order' using unique column(s)") + ApiRequestErrorCode09 + "A 'limit' was applied without an explicit 'order'" + Nothing + (Just "Apply an 'order' using unique column(s)") toJSON (OffLimitsChangesError n maxs) = toJsonPgrstError ApiRequestErrorCode10 @@ -475,13 +477,15 @@ pgErrorStatus authed (SQL.SessionUsageError (SQL.QueryError _ _ (SQL.ResultError '0':'9':_ -> HTTP.status500 -- triggered action exception '0':'L':_ -> HTTP.status403 -- invalid grantor '0':'P':_ -> HTTP.status403 -- invalid role specification - "23503" -> HTTP.status409 -- foreign_key_violation - "23505" -> HTTP.status409 -- unique_violation - "25006" -> HTTP.status405 -- read_only_sql_transaction "21000" -> -- cardinality_violation if BS.isSuffixOf "requires a WHERE clause" m then HTTP.status400 -- special case for pg-safeupdate, which we consider as client error else HTTP.status500 -- generic function or view server error, e.g. "more than one row returned by a subquery used as an expression" + "2201W" -> HTTP.status400 -- invalid/negative limit param + "2201X" -> HTTP.status400 -- invalid/negative offset param + "23503" -> HTTP.status409 -- foreign_key_violation + "23505" -> HTTP.status409 -- unique_violation + "25006" -> HTTP.status405 -- read_only_sql_transaction '2':'5':_ -> HTTP.status500 -- invalid tx state '2':'8':_ -> HTTP.status403 -- invalid auth specification '2':'D':_ -> HTTP.status500 -- invalid tx termination diff --git a/src/PostgREST/Plan.hs b/src/PostgREST/Plan.hs index db52d36332..844f8ab057 100644 --- a/src/PostgREST/Plan.hs +++ b/src/PostgREST/Plan.hs @@ -47,7 +47,6 @@ import PostgREST.Error (Error (..)) import PostgREST.MediaType (MediaType (..)) import PostgREST.Query.SqlFragment (sourceCTEName) import PostgREST.RangeQuery (NonnegRange, allRange, - convertToLimitZeroRange, restrictRange) import PostgREST.SchemaCache (SchemaCache (..)) import PostgREST.SchemaCache.Identifiers (FieldName, @@ -343,7 +342,9 @@ readPlan qi@QualifiedIdentifier{..} AppConfig{configDbMaxRows, configDbAggregate expandStars ctx =<< addRels qiSchema (iAction apiRequest) dbRelationships Nothing =<< addLogicTrees ctx apiRequest =<< - addRanges apiRequest =<< + addOffset apiRequest =<< + addLimit apiRequest =<< + addRange apiRequest =<< addOrders ctx apiRequest =<< addFilters ctx apiRequest (initReadRequest ctx $ QueryParams.qsSelect $ iQueryParams apiRequest) @@ -353,7 +354,7 @@ initReadRequest ctx@ResolverContext{qi=QualifiedIdentifier{..}} = foldr (treeEntry rootDepth) $ Node defReadPlan{from=qi ctx, relName=qiName, depth=rootDepth} [] where rootDepth = 0 - defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] allRange mempty Nothing [] Nothing mempty Nothing Nothing False [] rootDepth + defReadPlan = ReadPlan [] (QualifiedIdentifier mempty mempty) Nothing [] [] Nothing Nothing allRange mempty Nothing [] Nothing mempty Nothing Nothing False [] rootDepth treeEntry :: Depth -> Tree SelectItem -> ReadPlanTree -> ReadPlanTree treeEntry depth (Node si fldForest) (Node q rForest) = let nxtDepth = succ depth in @@ -456,7 +457,7 @@ treeRestrictRange _ (ActDb (ActRelationMut _ _)) request = Right request treeRestrictRange maxRows _ request = pure $ nodeRestrictRange maxRows <$> request where nodeRestrictRange :: Maybe Integer -> ReadPlan -> ReadPlan - nodeRestrictRange m q@ReadPlan{range_=r} = q{range_= convertToLimitZeroRange r (restrictRange m r) } + nodeRestrictRange m q@ReadPlan{range_=r} = q{range_= restrictRange m r } -- add relationships to the nodes of the tree by traversing the forest while keeping track of the parentNode(https://stackoverflow.com/questions/22721064/get-the-parent-of-a-node-in-data-tree-haskell#comment34627048_22721064) -- also adds aliasing @@ -791,7 +792,8 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do -- rootLabel = ReadPlan { -- select = [], -- there will be fields at this stage but we just omit them for brevity -- from = QualifiedIdentifier {qiSchema = "test", qiName = "projects"}, --- fromAlias = Just "projects_1", where_ = [], order = [], range_ = fullRange, +-- fromAlias = Just "projects_1", where_ = [], order = [], +-- offset = Nothing, limit = Nothing, range_ = fullRange, -- relName = "projects", -- relToParent = Nothing, -- relJoinConds = [], @@ -820,7 +822,8 @@ addRelatedOrders (Node rp@ReadPlan{order,from} forest) = do -- } -- ) -- ], --- order = [], range_ = fullRange, relName = "clients", relToParent = Nothing, relJoinConds = [], relAlias = Nothing, relAggAlias = "", relHint = Nothing, +-- order = [], offset = Nothing, limit = Nothing, range_ = fullRange, +-- relName = "clients", relToParent = Nothing, relJoinConds = [], relAlias = Nothing, relAggAlias = "", relHint = Nothing, -- relJoinType = Nothing, relIsSpread = False, depth = 0, -- relSelect = [] -- }, @@ -858,26 +861,38 @@ addNullEmbedFilters (Node rp@ReadPlan{where_=curLogic} forest) = do flt@(CoercibleStmnt _) -> Right flt -addRanges :: ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree -addRanges ApiRequest{..} rReq = +addOffset :: ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree +addOffset ApiRequest{..} rReq = + foldr addOffsetToNode (Right rReq) qsOffset + where + QueryParams.QueryParams{..} = iQueryParams + addOffsetToNode :: (EmbedPath, Integer) -> Either ApiRequestError ReadPlanTree ->Either ApiRequestError ReadPlanTree + addOffsetToNode = updateNode (\o (Node q f) -> Node q{offset=(Just o)} f) + +addLimit :: ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree +addLimit ApiRequest{..} rReq = + foldr addLimitToNode (Right rReq) qsLimit + where + QueryParams.QueryParams{..} = iQueryParams + addLimitToNode :: (EmbedPath, Integer) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree + addLimitToNode = updateNode (\l (Node q f) -> Node q{limit=(Just l)} f) + +addRange :: ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree +addRange ApiRequest{..} rReq = case iAction of ActDb (ActRelationMut _ _) -> Right rReq - _ -> foldr addRangeToNode (Right rReq) =<< ranges + _ -> foldr addRangeToNode (Right rReq) [([], iRange)] where - ranges :: Either ApiRequestError [(EmbedPath, NonnegRange)] - ranges = first QueryParamError $ QueryParams.pRequestRange `traverse` HM.toList iRange - addRangeToNode :: (EmbedPath, NonnegRange) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree addRangeToNode = updateNode (\r (Node q f) -> Node q{range_=r} f) addLogicTrees :: ResolverContext -> ApiRequest -> ReadPlanTree -> Either ApiRequestError ReadPlanTree addLogicTrees ctx ApiRequest{..} rReq = foldr addLogicTreeToNode (Right rReq) qsLogic - where - QueryParams.QueryParams{..} = iQueryParams - - addLogicTreeToNode :: (EmbedPath, LogicTree) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree - addLogicTreeToNode = updateNode (\t (Node q@ReadPlan{from=fromTable, where_=lf} f) -> Node q{ReadPlan.where_=resolveLogicTree ctx{qi=fromTable} t:lf} f) + where + QueryParams.QueryParams{..} = iQueryParams + addLogicTreeToNode :: (EmbedPath, LogicTree) -> Either ApiRequestError ReadPlanTree -> Either ApiRequestError ReadPlanTree + addLogicTreeToNode = updateNode (\t (Node q@ReadPlan{from=fromTable, where_=lf} f) -> Node q{ReadPlan.where_=resolveLogicTree ctx{qi=fromTable} t:lf} f) resolveLogicTree :: ResolverContext -> LogicTree -> CoercibleLogicTree resolveLogicTree ctx (Stmnt flt) = CoercibleStmnt $ resolveFilter ctx flt @@ -910,12 +925,12 @@ updateNode f (targetNodeName:remainingPath, a) (Right (Node rootNode forest)) = findNode = find (\(Node ReadPlan{relName, relAlias} _) -> relName == targetNodeName || relAlias == Just targetNodeName) forest mutatePlan :: Mutation -> QualifiedIdentifier -> ApiRequest -> SchemaCache -> ReadPlanTree -> Either Error MutatePlan -mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{dbTables, dbRepresentations} readReq = mapLeft ApiRequestError $ +mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{dbTables, dbRepresentations} readReq@(Node ReadPlan{offset,limit} _) = mapLeft ApiRequestError $ case mutation of MutationCreate -> mapRight (\typedColumns -> Insert qi typedColumns body ((,) <$> preferResolution <*> Just confCols) [] returnings pkCols applyDefaults) typedColumnsOrError MutationUpdate -> - mapRight (\typedColumns -> Update qi typedColumns body combinedLogic iTopLevelRange rootOrder returnings applyDefaults) typedColumnsOrError + mapRight (\typedColumns -> Update qi typedColumns body combinedLogic offset limit rootOrder returnings applyDefaults) typedColumnsOrError MutationSingleUpsert -> if null qsLogic && qsFilterFields == S.fromList pkCols && @@ -926,7 +941,7 @@ mutatePlan mutation qi ApiRequest{iPreferences=Preferences{..}, ..} SchemaCache{ then mapRight (\typedColumns -> Insert qi typedColumns body (Just (MergeDuplicates, pkCols)) combinedLogic returnings mempty False) typedColumnsOrError else Left InvalidFilters - MutationDelete -> Right $ Delete qi combinedLogic iTopLevelRange rootOrder returnings + MutationDelete -> Right $ Delete qi combinedLogic offset limit rootOrder returnings where ctx = ResolverContext dbTables dbRepresentations qi "json" confCols = fromMaybe pkCols qsOnConflict diff --git a/src/PostgREST/Plan/MutatePlan.hs b/src/PostgREST/Plan/MutatePlan.hs index 2e1b2e9cdc..b69357430b 100644 --- a/src/PostgREST/Plan/MutatePlan.hs +++ b/src/PostgREST/Plan/MutatePlan.hs @@ -1,5 +1,7 @@ +{-# LANGUAGE LambdaCase #-} module PostgREST.Plan.MutatePlan ( MutatePlan(..) + , mPlanLimit ) where @@ -9,7 +11,6 @@ import PostgREST.ApiRequest.Preferences (PreferResolution) import PostgREST.Plan.Types (CoercibleField, CoercibleLogicTree, CoercibleOrderTerm) -import PostgREST.RangeQuery (NonnegRange) import PostgREST.SchemaCache.Identifiers (FieldName, QualifiedIdentifier) @@ -32,7 +33,8 @@ data MutatePlan , updCols :: [CoercibleField] , updBody :: Maybe LBS.ByteString , where_ :: [CoercibleLogicTree] - , mutRange :: NonnegRange + , offset_ :: Maybe Integer + , limit_ :: Maybe Integer , mutOrder :: [CoercibleOrderTerm] , returning :: [FieldName] , applyDefs :: Bool @@ -40,7 +42,14 @@ data MutatePlan | Delete { in_ :: QualifiedIdentifier , where_ :: [CoercibleLogicTree] - , mutRange :: NonnegRange + , offset_ :: Maybe Integer + , limit_ :: Maybe Integer , mutOrder :: [CoercibleOrderTerm] , returning :: [FieldName] } + +mPlanLimit :: MutatePlan -> Maybe Integer +mPlanLimit = \case + Insert{} -> Nothing + Update{limit_=limit} -> limit + Delete{limit_=limit} -> limit diff --git a/src/PostgREST/Plan/ReadPlan.hs b/src/PostgREST/Plan/ReadPlan.hs index 854cf1ffa7..7b723abb47 100644 --- a/src/PostgREST/Plan/ReadPlan.hs +++ b/src/PostgREST/Plan/ReadPlan.hs @@ -34,6 +34,8 @@ data ReadPlan = ReadPlan , fromAlias :: Maybe Alias , where_ :: [CoercibleLogicTree] , order :: [CoercibleOrderTerm] + , offset :: Maybe Integer + , limit :: Maybe Integer , range_ :: NonnegRange , relName :: NodeName , relToParent :: Maybe Relationship diff --git a/src/PostgREST/Query.hs b/src/PostgREST/Query.hs index ab6ddf8cd5..8b193ae9aa 100644 --- a/src/PostgREST/Query.hs +++ b/src/PostgREST/Query.hs @@ -25,7 +25,6 @@ import qualified PostgREST.AppState as AppState import qualified PostgREST.Error as Error import qualified PostgREST.Query.QueryBuilder as QueryBuilder import qualified PostgREST.Query.Statements as Statements -import qualified PostgREST.RangeQuery as RangeQuery import qualified PostgREST.SchemaCache as SchemaCache import PostgREST.ApiRequest (ApiRequest (..), @@ -49,7 +48,7 @@ import PostgREST.Plan (ActionPlan (..), DbActionPlan (..), InfoPlan (..), InspectPlan (..)) -import PostgREST.Plan.MutatePlan (MutatePlan (..)) +import PostgREST.Plan.MutatePlan (MutatePlan (..), mPlanLimit) import PostgREST.Plan.ReadPlan (ReadPlanTree) import PostgREST.Query.SqlFragment (escapeIdentList, fromQi, intercalateSnippet, @@ -135,11 +134,11 @@ actionQuery (DbCrud plan@MutateReadPlan{mrMutation=MutationCreate, ..}) conf api optionalRollback conf apiReq pure $ DbCrudResult plan resultSet -actionQuery (DbCrud plan@MutateReadPlan{mrMutation=MutationUpdate, ..}) conf apiReq@ApiRequest{iPreferences=Preferences{..}, ..} _ _ = do +actionQuery (DbCrud plan@MutateReadPlan{mrMutation=MutationUpdate, ..}) conf apiReq@ApiRequest{iPreferences=Preferences{..}} _ _ = do resultSet <- writeQuery mrReadPlan mrMutatePlan mrMedia mrHandler apiReq conf failNotSingular mrMedia resultSet failExceedsMaxAffectedPref (preferMaxAffected,preferHandling) resultSet - failsChangesOffLimits (RangeQuery.rangeLimit iTopLevelRange) resultSet + failsChangesOffLimits (mPlanLimit mrMutatePlan) resultSet optionalRollback conf apiReq pure $ DbCrudResult plan resultSet @@ -149,11 +148,11 @@ actionQuery (DbCrud plan@MutateReadPlan{mrMutation=MutationSingleUpsert, ..}) co optionalRollback conf apiReq pure $ DbCrudResult plan resultSet -actionQuery (DbCrud plan@MutateReadPlan{mrMutation=MutationDelete, ..}) conf apiReq@ApiRequest{iPreferences=Preferences{..}, ..} _ _ = do +actionQuery (DbCrud plan@MutateReadPlan{mrMutation=MutationDelete, ..}) conf apiReq@ApiRequest{iPreferences=Preferences{..}} _ _ = do resultSet <- writeQuery mrReadPlan mrMutatePlan mrMedia mrHandler apiReq conf failNotSingular mrMedia resultSet failExceedsMaxAffectedPref (preferMaxAffected,preferHandling) resultSet - failsChangesOffLimits (RangeQuery.rangeLimit iTopLevelRange) resultSet + failsChangesOffLimits (mPlanLimit mrMutatePlan) resultSet optionalRollback conf apiReq pure $ DbCrudResult plan resultSet diff --git a/src/PostgREST/Query/QueryBuilder.hs b/src/PostgREST/Query/QueryBuilder.hs index 0d51c55484..39a129febe 100644 --- a/src/PostgREST/Query/QueryBuilder.hs +++ b/src/PostgREST/Query/QueryBuilder.hs @@ -37,13 +37,12 @@ import PostgREST.Plan.MutatePlan import PostgREST.Plan.ReadPlan import PostgREST.Plan.Types import PostgREST.Query.SqlFragment -import PostgREST.RangeQuery (allRange) import Protolude readPlanToQuery :: ReadPlanTree -> SQL.Snippet -readPlanToQuery node@(Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,order, range_=readRange, relToParent, relJoinConds, relSelect} forest) = - "SELECT " <> +readPlanToQuery node@(Node ReadPlan{select,from=mainQi,fromAlias,where_=logicForest,order,offset,limit,range_=readRange, relToParent, relJoinConds, relSelect} forest) = + "WITH pgrst_select_body AS ( SELECT " <> intercalateSnippet ", " ((pgFmtSelectItem qi <$> (if null select && null forest then defSelect else select)) ++ joinsSelects) <> " " <> fromFrag <> " " <> intercalateSnippet " " joins <> " " <> @@ -52,8 +51,12 @@ readPlanToQuery node@(Node ReadPlan{select,from=mainQi,fromAlias,where_=logicFor else "WHERE " <> intercalateSnippet " AND " (map (pgFmtLogicTree qi) logicForest ++ map pgFmtJoinCondition relJoinConds)) <> " " <> groupF qi select relSelect <> " " <> orderF qi order <> " " <> - limitOffsetF readRange + offsetF <> " " <> limitF <> " ) " <> + "SELECT * FROM pgrst_select_body " <> + rangeHeaderF readRange where + limitF = maybe mempty (\x -> "LIMIT " <> intToSqlSnip x) limit + offsetF = maybe mempty (\x -> "OFFSET " <> intToSqlSnip x) offset fromFrag = fromF relToParent mainQi fromAlias qi = getQualifiedIdentifier relToParent mainQi fromAlias -- gets all the columns in case of an empty select, ignoring/obtaining these columns is done at the aggregation stage @@ -132,14 +135,14 @@ mutatePlanToQuery (Insert mainQi iCols body onConflct putConditions returnings _ mergeDups = case onConflct of {Just (MergeDuplicates,_) -> True; _ -> False;} -- An update without a limit is always filtered with a WHERE -mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings applyDefaults) +mutatePlanToQuery (Update mainQi uCols body logicForest offset limit ordts returnings applyDefaults) | null uCols = -- if there are no columns we cannot do UPDATE table SET {empty}, it'd be invalid syntax -- selecting an empty resultset from mainQi gives us the column names to prevent errors when using &select= -- the select has to be based on "returnings" to make computed overloaded functions not throw "SELECT " <> emptyBodyReturnedColumns <> " FROM " <> fromQi mainQi <> " WHERE false" - | range == allRange = + | isNothing offset && isNothing limit = "UPDATE " <> mainTbl <> " SET " <> nonRangeCols <> " " <> fromJsonBodyF body uCols False False applyDefaults <> whereLogic <> " " <> @@ -152,7 +155,7 @@ mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings a "SELECT " <> rangeIdF <> " FROM " <> mainTbl <> whereLogic <> " " <> orderF mainQi ordts <> " " <> - limitOffsetF range <> + offsetF <> " " <> limitF <> " " <> ") " <> "UPDATE " <> mainTbl <> " SET " <> rangeCols <> "FROM pgrst_affected_rows " <> @@ -160,6 +163,8 @@ mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings a returningF mainQi returnings where + limitF = maybe mempty (\x -> "LIMIT " <> intToSqlSnip x) limit + offsetF = maybe mempty (\x -> "OFFSET " <> intToSqlSnip x) offset whereLogic = if null logicForest then mempty else " WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree mainQi <$> logicForest) mainTbl = fromQi mainQi emptyBodyReturnedColumns = if null returnings then "NULL" else intercalateSnippet ", " (pgFmtColumn (QualifiedIdentifier mempty $ qiName mainQi) <$> returnings) @@ -167,8 +172,8 @@ mutatePlanToQuery (Update mainQi uCols body logicForest range ordts returnings a rangeCols = intercalateSnippet ", " ((\col -> pgFmtIdent (cfName col) <> " = (SELECT " <> pgFmtIdent (cfName col) <> " FROM pgrst_update_body) ") <$> uCols) (whereRangeIdF, rangeIdF) = mutRangeF mainQi (cfName . coField <$> ordts) -mutatePlanToQuery (Delete mainQi logicForest range ordts returnings) - | range == allRange = +mutatePlanToQuery (Delete mainQi logicForest offset limit ordts returnings) + | isNothing offset && isNothing limit = "DELETE FROM " <> fromQi mainQi <> " " <> whereLogic <> " " <> returningF mainQi returnings @@ -179,14 +184,16 @@ mutatePlanToQuery (Delete mainQi logicForest range ordts returnings) "SELECT " <> rangeIdF <> " FROM " <> fromQi mainQi <> whereLogic <> " " <> orderF mainQi ordts <> " " <> - limitOffsetF range <> - ") " <> + offsetF <> " " <> limitF <> + " ) " <> "DELETE FROM " <> fromQi mainQi <> " " <> "USING pgrst_affected_rows " <> "WHERE " <> whereRangeIdF <> " " <> returningF mainQi returnings where + limitF = maybe mempty (\x -> "LIMIT " <> intToSqlSnip x) limit + offsetF = maybe mempty (\x -> "OFFSET " <> intToSqlSnip x) offset whereLogic = if null logicForest then mempty else " WHERE " <> intercalateSnippet " AND " (pgFmtLogicTree mainQi <$> logicForest) (whereRangeIdF, rangeIdF) = mutRangeF mainQi (cfName . coField <$> ordts) diff --git a/src/PostgREST/Query/SqlFragment.hs b/src/PostgREST/Query/SqlFragment.hs index bc6e483661..073d0f2bb1 100644 --- a/src/PostgREST/Query/SqlFragment.hs +++ b/src/PostgREST/Query/SqlFragment.hs @@ -11,7 +11,7 @@ module PostgREST.Query.SqlFragment , countF , groupF , fromQi - , limitOffsetF + , rangeHeaderF , locationF , mutRangeF , orderF @@ -33,6 +33,7 @@ module PostgREST.Query.SqlFragment , sourceCTE , sourceCTEName , unknownEncoder + , intToSqlSnip , intercalateSnippet , explainF , setConfigWithConstantName @@ -479,12 +480,12 @@ returningF qi returnings = then "RETURNING 1" -- For mutation cases where there's no ?select, we return 1 to know how many rows were modified else "RETURNING " <> intercalateSnippet ", " (pgFmtColumn qi <$> returnings) -limitOffsetF :: NonnegRange -> SQL.Snippet -limitOffsetF range = +rangeHeaderF :: NonnegRange -> SQL.Snippet +rangeHeaderF range = if range == allRange then mempty else "LIMIT " <> limit <> " OFFSET " <> offset where - limit = maybe "ALL" (\l -> unknownEncoder (BS.pack $ show l)) $ rangeLimit range - offset = unknownEncoder (BS.pack . show $ rangeOffset range) + limit = maybe "ALL" intToSqlSnip $ rangeLimit range + offset = intToSqlSnip $ rangeOffset range responseHeadersF :: SQL.Snippet responseHeadersF = currentSettingF "response.headers" @@ -520,6 +521,9 @@ unknownEncoder = SQL.encoderAndParam (HE.nonNullable HE.unknown) unknownLiteral :: Text -> SQL.Snippet unknownLiteral = unknownEncoder . encodeUtf8 +intToSqlSnip :: Integer -> SQL.Snippet +intToSqlSnip x = unknownEncoder (BS.pack $ show x) + intercalateSnippet :: ByteString -> [SQL.Snippet] -> SQL.Snippet intercalateSnippet _ [] = mempty intercalateSnippet frag snippets = foldr1 (\a b -> a <> SQL.sql frag <> b) snippets diff --git a/src/PostgREST/RangeQuery.hs b/src/PostgREST/RangeQuery.hs index a27a1af5cd..767fc8c74d 100644 --- a/src/PostgREST/RangeQuery.hs +++ b/src/PostgREST/RangeQuery.hs @@ -99,7 +99,7 @@ rangeStatusHeader topLevelRange queryTotal tableTotal = upper = lower + toInteger queryTotal - 1 contentRange = contentRangeH lower upper (toInteger <$> tableTotal) status = rangeStatus lower upper (toInteger <$> tableTotal) - in (status, contentRange) + in (if topLevelRange == allRange then status200 else status, contentRange) where rangeStatus :: Integer -> Integer -> Maybe Integer -> Status rangeStatus _ _ Nothing = status200 diff --git a/src/PostgREST/Response.hs b/src/PostgREST/Response.hs index d50f825ba2..77a869397c 100644 --- a/src/PostgREST/Response.hs +++ b/src/PostgREST/Response.hs @@ -68,7 +68,7 @@ actionResponse (DbCrudResult WrappedReadPlan{wrMedia, wrHdrsOnly=headersOnly, cr case resultSet of RSStandard{..} -> do let - (status, contentRange) = RangeQuery.rangeStatusHeader iTopLevelRange rsQueryTotal rsTableTotal + (status, contentRange) = RangeQuery.rangeStatusHeader iRange rsQueryTotal rsTableTotal prefHeader = maybeToList . prefAppliedHeader $ Preferences Nothing Nothing Nothing preferCount preferTransaction Nothing preferHandling preferTimezone Nothing [] headers = [ contentRange @@ -84,7 +84,7 @@ actionResponse (DbCrudResult WrappedReadPlan{wrMedia, wrHdrsOnly=headersOnly, cr (ovStatus, ovHeaders) <- overrideStatusHeaders rsGucStatus rsGucHeaders status headers let bod | status == HTTP.status416 = Error.errorPayload $ Error.ApiRequestError $ ApiRequestTypes.InvalidRange $ - ApiRequestTypes.OutOfBounds (show $ RangeQuery.rangeOffset iTopLevelRange) (maybe "0" show rsTableTotal) + ApiRequestTypes.OutOfBounds (show $ RangeQuery.rangeOffset iRange) (maybe "0" show rsTableTotal) | headersOnly = mempty | otherwise = LBS.fromStrict rsBody @@ -201,10 +201,10 @@ actionResponse (DbCallResult CallReadPlan{crMedia, crInvMthd=invMethod, crProc=p RSStandard {..} -> do let (status, contentRange) = - RangeQuery.rangeStatusHeader iTopLevelRange rsQueryTotal rsTableTotal + RangeQuery.rangeStatusHeader iRange rsQueryTotal rsTableTotal rsOrErrBody = if status == HTTP.status416 then Error.errorPayload $ Error.ApiRequestError $ ApiRequestTypes.InvalidRange - $ ApiRequestTypes.OutOfBounds (show $ RangeQuery.rangeOffset iTopLevelRange) (maybe "0" show rsTableTotal) + $ ApiRequestTypes.OutOfBounds (show $ RangeQuery.rangeOffset iRange) (maybe "0" show rsTableTotal) else LBS.fromStrict rsBody prefHeader = maybeToList . prefAppliedHeader $ Preferences Nothing Nothing preferParameters preferCount preferTransaction Nothing preferHandling preferTimezone preferMaxAffected [] headers = contentRange : prefHeader diff --git a/test/spec/Feature/Query/QueryLimitedSpec.hs b/test/spec/Feature/Query/QueryLimitedSpec.hs index 251c319814..9315fa0c34 100644 --- a/test/spec/Feature/Query/QueryLimitedSpec.hs +++ b/test/spec/Feature/Query/QueryLimitedSpec.hs @@ -52,7 +52,7 @@ spec = "" `shouldRespondWith` "" - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-1/2019" ] } @@ -73,7 +73,7 @@ spec = "" `shouldRespondWith` "" - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-1/3" ] } diff --git a/test/spec/Feature/Query/QuerySpec.hs b/test/spec/Feature/Query/QuerySpec.hs index 3b1da167b6..88abc612eb 100644 --- a/test/spec/Feature/Query/QuerySpec.hs +++ b/test/spec/Feature/Query/QuerySpec.hs @@ -754,10 +754,7 @@ spec actualPgVersion = do it "can embed a view that has group by" $ get "/projects_count_grouped_by?select=number_of_projects,client:clients(name)&order=number_of_projects" `shouldRespondWith` - [json| - [{"number_of_projects":1,"client":null}, - {"number_of_projects":2,"client":{"name":"Microsoft"}}, - {"number_of_projects":2,"client":{"name":"Apple"}}] |] + [json|[{"number_of_projects":1,"client":null},{"number_of_projects":2,"client":{"name": "Apple"}},{"number_of_projects":2,"client":{"name": "Microsoft"}}] |] { matchHeaders = [matchContentTypeJson] } it "can embed a view that has a subselect containing a select in a where" $ diff --git a/test/spec/Feature/Query/RangeSpec.hs b/test/spec/Feature/Query/RangeSpec.hs index 5eb2e17740..2567e245dd 100644 --- a/test/spec/Feature/Query/RangeSpec.hs +++ b/test/spec/Feature/Query/RangeSpec.hs @@ -30,32 +30,22 @@ spec = do [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}] |] { matchHeaders = ["Content-Range" <:> "0-14/*"] } - context "of invalid range" $ do - it "refuses a range with nonzero start when there are no items" $ + context "offset exceeding total rows" $ do + it "returns nothing when offset greater than 0 for when result is empty" $ request methodGet "/rpc/getitemrange?offset=1&min=2&max=2" [("Prefer", "count=exact")] mempty `shouldRespondWith` - [json| { - "message":"Requested range not satisfiable", - "code":"PGRST103", - "details":"An offset of 1 was requested, but there are only 0 rows.", - "hint":null - }|] - { matchStatus = 416 + [json|[]|] + { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "*/0"] } - it "refuses a range requesting start past last item" $ + it "returns nothing when offset exceeds total rows" $ request methodGet "/rpc/getitemrange?offset=100&min=0&max=15" [("Prefer", "count=exact")] mempty `shouldRespondWith` - [json| { - "message":"Requested range not satisfiable", - "code":"PGRST103", - "details":"An offset of 100 was requested, but there are only 15 rows.", - "hint":null - }|] - { matchStatus = 416 + [json|[]|] + { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "*/15"] } @@ -238,11 +228,12 @@ spec = do [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}]|] { matchHeaders = ["Content-Range" <:> "0-14/*"] } - it "succeeds if offset is negative as a no-op" $ + it "fails with pg error if offset is negative" $ get "/items?select=id&offset=-4&order=id" `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}]|] - { matchHeaders = ["Content-Range" <:> "0-14/*"] } + [json|{"code":"2201X","details":null,"hint":null,"message":"OFFSET must not be negative"} |] + { matchStatus = 400 + , matchHeaders = [matchContentTypeJson] } it "succeeds and returns an empty array if limit equals 0" $ get "/items?select=id&limit=0" @@ -254,45 +245,11 @@ spec = do it "fails if limit is negative" $ get "/items?select=id&limit=-1" `shouldRespondWith` - [json| { - "message":"Requested range not satisfiable", - "code":"PGRST103", - "details":"Limit should be greater than or equal to zero.", - "hint":null - }|] - { matchStatus = 416 + [json|{"code":"2201W","details":null,"hint":null,"message":"LIMIT must not be negative"} |] + { matchStatus = 400 , matchHeaders = [matchContentTypeJson] } - context "of invalid range" $ do - it "refuses a range with nonzero start when there are no items" $ - request methodGet "/menagerie?offset=1" - [("Prefer", "count=exact")] "" - `shouldRespondWith` - [json| { - "message":"Requested range not satisfiable", - "code":"PGRST103", - "details":"An offset of 1 was requested, but there are only 0 rows.", - "hint":null - }|] - { matchStatus = 416 - , matchHeaders = ["Content-Range" <:> "*/0"] - } - - it "refuses a range requesting start past last item" $ - request methodGet "/items?offset=100" - [("Prefer", "count=exact")] "" - `shouldRespondWith` - [json| { - "message":"Requested range not satisfiable", - "code":"PGRST103", - "details":"An offset of 100 was requested, but there are only 15 rows.", - "hint":null - }|] - { matchStatus = 416 - , matchHeaders = ["Content-Range" <:> "*/15"] - } - context "when count=planned" $ do it "obtains a filtered range" $ do request methodGet "/items?select=id&id=gt.8" @@ -300,7 +257,7 @@ spec = do "" `shouldRespondWith` [json|[{"id":9}, {"id":10}, {"id":11}, {"id":12}, {"id":13}, {"id":14}, {"id":15}]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "0-6/8"] } @@ -309,7 +266,7 @@ spec = do "" `shouldRespondWith` [json|[{"id":4}, {"id":5}, {"id":6}]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "0-2/4"] } @@ -318,7 +275,7 @@ spec = do "" `shouldRespondWith` [json|[{"id":1}, {"id":2}]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "0-1/673"] } @@ -348,7 +305,7 @@ spec = do "" `shouldRespondWith` "" - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-4/2019" ] } diff --git a/test/spec/Feature/Query/RelatedQueriesSpec.hs b/test/spec/Feature/Query/RelatedQueriesSpec.hs index de0dd8f2d0..4e18a70cee 100644 --- a/test/spec/Feature/Query/RelatedQueriesSpec.hs +++ b/test/spec/Feature/Query/RelatedQueriesSpec.hs @@ -321,7 +321,7 @@ spec = describe "related queries" $ do {"name":"IOS", "clients":{"name":"Apple"}}, {"name":"OSX", "clients":{"name":"Apple"}} ]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-3/1200" ] } @@ -340,7 +340,7 @@ spec = describe "related queries" $ do {"id":1,"name":"Walmart"}, {"id":2,"name":"Target"} ]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-1/952" ] } @@ -355,7 +355,7 @@ spec = describe "related queries" $ do {"name":"IOS", "clients":{"name":"Apple"}}, {"name":"OSX", "clients":{"name":"Apple"}} ]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-3/1200" ] } @@ -374,7 +374,7 @@ spec = describe "related queries" $ do {"id":1,"name":"Walmart"}, {"id":2,"name":"Target"} ]|] - { matchStatus = 206 + { matchStatus = 200 , matchHeaders = [ matchContentTypeJson , "Content-Range" <:> "0-1/952" ] } diff --git a/test/spec/Feature/Query/RpcSpec.hs b/test/spec/Feature/Query/RpcSpec.hs index 0e4af838e2..038db96acc 100644 --- a/test/spec/Feature/Query/RpcSpec.hs +++ b/test/spec/Feature/Query/RpcSpec.hs @@ -64,7 +64,7 @@ spec actualPgVersion = [("Prefer", "count=exact")] [json| { "min": 2, "max": 4 } |] `shouldRespondWith` [json| [{"id":4}] |] - { matchStatus = 206 -- it now knows the response is partial + { matchStatus = 200 , matchHeaders = ["Content-Range" <:> "1-1/2"] } request methodGet "/rpc/getitemrange?min=2&max=4&limit=1&offset=1"