suggested cleaup by @begriffs

CHANGELOG.md

@@ -12,7 +12,7 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Accept posts from HTML forms - @begriffs
 - Ability to order embedded entities - @ruslantalpa
 - Ability to paginate using &limit and &offset parameters - @ruslantalpa
-- Ability to apply limits to embedded entities and enforce --max-rows on all levels - @ruslantalpa 
+- Ability to apply limits to embedded entities and enforce --max-rows on all levels - @ruslantalpa, @begriffs 
 
 ### Fixed
 - Return 401 or 403 for access denied rather than 404 - @begriffs

docs/api/reading.md

@@ -174,7 +174,7 @@ GET /people?order=age.desc.nullslast
 
 To order the embedded items, you need to specify the tree path for the order param like so.
 ```HTTP
-GET /projects?select=id,name,tasks{id,name}&order=id.ask&tasks.order=name.ask
+GET /projects?select=id,name,tasks{id,name}&order=id.asc&tasks.order=name.asc
 ```
 
 

src/PostgREST/ApiRequest.hs

@@ -15,17 +15,18 @@ import           Data.Monoid               ((<>))
 import           Data.Ord                  (comparing)
 import           Data.String.Conversions   (cs)
 import qualified Data.Text                 as T
+import           Text.Read                 (readMaybe)
 import qualified Data.Vector               as V
 import           Network.HTTP.Base         (urlEncodeVars)
 import           Network.HTTP.Types.Header (hAuthorization)
 import           Network.HTTP.Types.URI    (parseSimpleQuery)
 import           Network.Wai               (Request (..))
 import           Network.Wai.Parse         (parseHttpAccept)
-import           PostgREST.RangeQuery      (NonnegRange, rangeRequested, limitToRange, toRange )
+import           PostgREST.RangeQuery      (NonnegRange, rangeRequested, restrictRange, rangeGeq, allRange)
 import           PostgREST.Types           (QualifiedIdentifier (..),
                                             Schema, Payload(..),
                                             UniformObjects(..))
-import           Data.Ranged.Ranges        (singletonRange)
+import           Data.Ranged.Ranges        (singletonRange, rangeIntersection)
 
 type RequestBody = BL.ByteString
 
@@ -141,9 +142,8 @@ userApiRequest schema req reqBody =
   ApiRequest {
     iAction = action
   , iTarget = target
-  , iRange = setTopLevelRange headerRange $
+  , iRange = M.insert "limit" (rangeIntersection headerRange urlRange) $
-             setTopLevelRange urlRange $
+      M.fromList [ (cs k, restrictRange (readMaybe =<< v) allRange) | (k,v) <- qParams, isJust v, endingIn ["limit"] k ]
-             M.fromList [(cs k, limitToRange $ cs $ fromJust v) | (k,v) <- qParams, isJust v, endingIn ["limit"] k ]
   , iAccepts = pickContentType $ lookupHeader "accept"
   , iPayload = relevantPayload
   , iPreferRepresentation = representation
@@ -187,13 +187,12 @@ userApiRequest schema req reqBody =
   endingIn xx key = lastWord `elem` xx
     where lastWord = last $ T.split (=='.') key
 
-  headerRange = if singular then Just (singletonRange 0) else rangeRequested hdrs
+  headerRange = if singular then singletonRange 0 else rangeRequested hdrs
-  urlRange = toRange (join $ lookup "limit" qParams) (join $ lookup "offset" qParams)
+  urlOffsetRange = rangeGeq . fromMaybe (0::Integer) $
-
+    readMaybe =<< join (lookup "offset" qParams)
-  setTopLevelRange :: Maybe NonnegRange -> M.HashMap String NonnegRange -> M.HashMap String NonnegRange
+  urlRange = restrictRange
-  setTopLevelRange Nothing ranges = ranges
+    (readMaybe =<< join (lookup "limit" qParams))
-  setTopLevelRange (Just r) ranges = M.insert "limit" r ranges
+    urlOffsetRange
-
 
 -- PRIVATE ---------------------------------------------------------------
 

src/PostgREST/App.hs

@@ -47,7 +47,7 @@ import           PostgREST.Config          (AppConfig (..))
 import           PostgREST.DbStructure
 import           PostgREST.Error           (errResponse, pgErrResponse)
 import           PostgREST.Parsers
-import           PostgREST.RangeQuery
+import           PostgREST.RangeQuery      (NonnegRange, allRange, rangeOffset, restrictRange)
 import           PostgREST.Middleware
 import           PostgREST.QueryBuilder ( callProc
                                         , addJoinConditions
@@ -221,7 +221,7 @@ app dbStructure conf apiRequest =
   allOrigins = ("Access-Control-Allow-Origin", "*") :: Header
   schema = cs $ configSchema conf
   shouldCount = iPreferCount apiRequest
-  topLevelRange = fromMaybe (rangeGeq 0) $ M.lookup "limit" $ iRange apiRequest
+  topLevelRange = fromMaybe allRange $ M.lookup "limit" $ iRange apiRequest
   readDbRequest = DbRead <$> buildReadRequest (configMaxRows conf) (dbRelations dbStructure) apiRequest
   mutateDbRequest = DbMutate <$> buildMutateRequest apiRequest
   selectQuery = requestToQuery schema <$> readDbRequest
@@ -379,7 +379,7 @@ addOrder :: (Path, [OrderTerm]) -> ReadRequest -> ReadRequest
 addOrder = addProperty addOrderToNode
 
 addRangeToNode :: NonnegRange -> ReadRequest -> ReadRequest
-addRangeToNode r (Node (q,i) f) = Node (q{range_=Just r}, i) f
+addRangeToNode r (Node (q,i) f) = Node (q{range_=r}, i) f
 
 addRange :: (Path, NonnegRange) -> ReadRequest -> ReadRequest
 addRange = addProperty addRangeToNode

src/PostgREST/Parsers.hs

@@ -11,15 +11,14 @@ import           Data.Tree
 import           PostgREST.QueryBuilder        (operators)
 import           PostgREST.Types
 import           Text.ParserCombinators.Parsec hiding (many, (<|>))
-import           PostgREST.RangeQuery      (NonnegRange)
+import           PostgREST.RangeQuery      (NonnegRange,allRange)
-
 
 pRequestSelect :: Text -> Parser ReadRequest
 pRequestSelect rootNodeName = do
   fieldTree <- pFieldForest
   return $ foldr treeEntry (Node (readQuery, (rootNodeName, Nothing, Nothing)) []) fieldTree
   where
-    readQuery = Select [] [rootNodeName] [] Nothing Nothing
+    readQuery = Select [] [rootNodeName] [] Nothing allRange
     treeEntry :: Tree SelectItem -> ReadRequest -> ReadRequest
     treeEntry (Node fld@((fn, _),_,alias) fldForest) (Node (q, i) rForest) =
       case fldForest of
@@ -27,7 +26,7 @@ pRequestSelect rootNodeName = do
         _  -> Node (q, i) newForest
           where
             newForest =
-              foldr treeEntry (Node (Select [] [fn] [] Nothing Nothing, (fn, Nothing, alias)) []) fldForest:rForest
+              foldr treeEntry (Node (Select [] [fn] [] Nothing allRange, (fn, Nothing, alias)) []) fldForest:rForest
 
 pRequestFilter :: (String, String) -> Either ParseError (Path, Filter)
 pRequestFilter (k, v) = (,) <$> path <*> (Filter <$> fld <*> op <*> val)

src/PostgREST/QueryBuilder.hs

@@ -35,7 +35,7 @@ import qualified Hasql.Decoders          as HD
 import qualified Data.Aeson              as JSON
 import           Data.Int                (Int64)
 
-import           PostgREST.RangeQuery    (NonnegRange, rangeLimit, rangeOffset)
+import           PostgREST.RangeQuery    (NonnegRange, rangeLimit, rangeOffset, allRange)
 import           Control.Error           (note, fromMaybe)
 import           Data.Functor.Contravariant (contramap)
 import qualified Data.HashMap.Strict     as HM
@@ -289,7 +289,7 @@ requestToQuery schema (DbRead (Node (Select colSelects tbls conditions ord range
       unwords joins,
       ("WHERE " <> intercalate " AND " ( map (pgFmtCondition qi ) conditions )) `emptyOnNull` conditions,
       orderF (fromMaybe [] ord),
-      fromMaybe "" $ limitF <$> range
+      limitF range
       ]
     orderF ts =
         if null ts
@@ -409,7 +409,9 @@ locationF pKeys =
     ) <> ")"
 
 limitF :: NonnegRange -> SqlFragment
-limitF r  = "LIMIT " <> limit <> " OFFSET " <> offset
+limitF r  = if r == allRange
+  then ""
+  else "LIMIT " <> limit <> " OFFSET " <> offset
   where
     limit  = maybe "ALL" (cs . show) $ rangeLimit r
     offset = cs . show $ rangeOffset r

src/PostgREST/RangeQuery.hs

@@ -5,15 +5,13 @@ module PostgREST.RangeQuery (
 , rangeOffset
 , restrictRange
 , rangeGeq
+, allRange
 , NonnegRange
-, limitToRange
-, toRange
 ) where
 
 
 import           Control.Applicative
 import           Network.HTTP.Types.Header
-import           Data.Monoid               ((<>))
 
 import qualified Data.ByteString.Char8     as BS
 import           Data.Ranged.Boundaries
@@ -37,19 +35,19 @@ rangeParse range = do
     Just parsedRange ->
       let [_, from, to] = readMaybe . cs <$> parsedRange
           lower         = fromMaybe emptyRange   (rangeGeq <$> from)
-          upper         = fromMaybe (rangeGeq 0) (rangeLeq <$> to) in
+          upper         = fromMaybe allRange (rangeLeq <$> to) in
       rangeIntersection lower upper
-    Nothing -> rangeGeq 0
+    Nothing -> allRange
 
-rangeRequested :: RequestHeaders -> Maybe NonnegRange
+rangeRequested :: RequestHeaders -> NonnegRange
-rangeRequested headers = rangeParse <$> lookup hRange headers
+rangeRequested headers = fromMaybe allRange $
+  rangeParse <$> lookup hRange headers
 
-restrictRange :: Maybe Integer -> Maybe NonnegRange -> Maybe NonnegRange
+restrictRange :: Maybe Integer -> NonnegRange -> NonnegRange
 restrictRange Nothing r = r
-restrictRange (Just limit) Nothing = Just $ rangeIntersection (rangeGeq 0) (rangeLeq (limit - 1))
+restrictRange (Just limit) r =
-restrictRange (Just limit) (Just r) = Just $
+   rangeIntersection r $
-  rangeIntersection r $
+     Range BoundaryBelowAll (BoundaryAbove $ rangeOffset r + limit - 1)
-    Range BoundaryBelowAll (BoundaryAbove $ rangeOffset r + limit - 1)
 
 rangeLimit :: NonnegRange -> Maybe Integer
 rangeLimit range =
@@ -67,20 +65,9 @@ rangeGeq :: Integer -> NonnegRange
 rangeGeq n =
   Range (BoundaryBelow n) BoundaryAboveAll
 
+allRange :: NonnegRange
+allRange = rangeGeq 0
+
 rangeLeq :: Integer -> NonnegRange
 rangeLeq n =
   Range BoundaryBelowAll (BoundaryAbove n)
-
-limitToRange :: BS.ByteString -> NonnegRange
-limitToRange l = rangeParse ("0-" <> cs (show (l' - 1)))
-  where l' = fromMaybe 0 (readMaybe $ cs l)::Integer
-
-toRange :: Maybe String -> Maybe String -> Maybe NonnegRange
-toRange Nothing Nothing = Nothing
-toRange Nothing (Just o) = Just $ rangeParse $ cs $ show o' <> "-"
-  where o' = fromMaybe 0 (readMaybe $ cs o)::Integer
-toRange (Just l) Nothing = Just $ limitToRange $ cs l
-toRange (Just l) (Just o) = Just $ rangeParse $ cs $ show o' <> "-" <> show (o' + l' - 1)
-  where
-    l' = fromMaybe 0 (readMaybe $ cs l)::Integer
-    o' = fromMaybe 0 (readMaybe $ cs o)::Integer

src/PostgREST/Types.hs

@@ -112,7 +112,7 @@ type Cast = Text
 type NodeName = Text
 type SelectItem = (Field, Maybe Cast, Maybe Alias)
 type Path = [Text]
-data ReadQuery = Select { select::[SelectItem], from::[TableName], flt_::[Filter], order::Maybe [OrderTerm], range_::Maybe NonnegRange } deriving (Show, Eq)
+data ReadQuery = Select { select::[SelectItem], from::[TableName], flt_::[Filter], order::Maybe [OrderTerm], range_::NonnegRange } deriving (Show, Eq)
 data MutateQuery = Insert { in_::TableName, qPayload::Payload }
                  | Delete { in_::TableName, where_::[Filter] }
                  | Update { in_::TableName, qPayload::Payload, where_::[Filter] } deriving (Show, Eq)