silkapp / rest

Packages for defining APIs, running them, generating client code and documentation.
http://silkapp.github.io/rest
390 stars 52 forks source link

Remove hard-limit on collection size #119

Open Philonous opened 9 years ago

Philonous commented 9 years ago

List Handlers using the default Range parameter have a build in upper limit of 1000 items returned. I've tried working around it with genHandler and a custom parameter, but it seems that rest-core enforces a hard-limit of 1000 on items returned and will clip the list if it is longer. This limit seems arbitrary and makes it harder to retrieve large collections wholesale when no pagination is needed (e.g. for statistical analysis).

I suggest either removing the list clipping behaviour or restricting it to cases where an explicit upper limit is given.

The offending code seems to be here: https://github.com/silkapp/rest/blob/edc00f966d3d406f9f66c23ebea984ba3c67539c/rest-core/src/Rest/Driver/Routing.hs#L307

jonkri commented 9 years ago

+1

hesselink commented 9 years ago

I agree that this is rather arbitrary. However, you can easily work around it, as this is just the default list handler, you can easily make your own for the time being. The relevant code is actually here. If you create your own parameter parser that has different bounds or none, everything else should still work.

In the long term, we could either remove the bounds completely, make them configurable, or offer two list handlers. @bergmark What do you think?

Philonous commented 9 years ago

I tried that and the 1000-item hard-limit still seems to be enforced.

Skimming the code it seems that a ListHandler is converted to a Handler internally (using the defaul Range parameter parsing, even when I use a customer parameter ), clipping the list in the process.

Philonous commented 9 years ago

More specifically, my custom parameter doesn't set default values (rather, it leaves the values to be Nothing if unset, in which case all items are returned), and the internal conversion limits the number of items to 100 in this case.

hesselink commented 9 years ago

Hmm, it seems you're right. It looks like the routing code is adding the default parameters to the list handler for some reason, probably to be able to do the slicing. I'll think about how to fix this, it doesn't look as straightforward as I initially thought.

bergmark commented 9 years ago

I agree that the bound is arbitrary and shouldn't be implicit. I actually ran across this the other day as well and was confused.

I see these options for bounded listhandlers

  1. Let users define it themselves
  2. Add a api-global setting for the limit
  3. Add a mkBoundedListing :: Int -> ...
  4. Add a mk1000BoundedListing

Option 4 Is still arbitrary so no thanks. 1 will always be possible. I'm fine with either 2 or 3, we could also do both.

bergmark commented 9 years ago

Another limitation of listings is that you have to use the predefined query parameters. offset is not always possible to respond to. In one case I want to use before or after dates instead.

hesselink commented 9 years ago

Isn't it possible to do that using a custom handler (using mkGenHandler)?

bergmark commented 9 years ago

Hmm yeah I think so

jonkri commented 9 years ago

It would also be nice if the library could support a way to inform how many items there are in total. This would make pagination easier, since the list call would include all the information necessary to figure out how many pages there are.

bergmark commented 8 years ago

I ended up here again...

It's not possible to use mkGenHandler to define a ListHandler with an increased limit, it will still be truncated.

I think the only way to work around this is to skip the list field for the resource and use statics instead.

bergmark commented 8 years ago

I ended up using this for now

#!/usr/bin/env stack
-- stack --resolver lts-6.10 --install-ghc runghc
{-# LANGUAGE DataKinds #-}
module UnlimitedListing where

import           Control.Monad.Except (ExceptT)
import           Rest                 (DataError (ParseError), Handler,
                                       Range (..), Reason, Void, mkHandler,
                                       mkPar, param)
import           Rest.Dictionary      (Dict, FromMaybe, Param (Param))
import           Text.Read            (readMaybe)

-- | A version of 'Rest.Handler.mkListing' with no upper limit for the
-- @count@ parameter.
--
-- Be careful not to use this for listings that are expensive to
-- compute (and if you use mkListing, enforce a maximum yourself).
mkUnlimitedListing :: Monad m
  => (Dict () () 'Nothing 'Nothing 'Nothing
  -> Dict h x i' o' e')
  -> (Range -> ExceptT (Reason (FromMaybe Void e')) m (FromMaybe () o'))
  -> Handler m
mkUnlimitedListing d a = mkHandler (mkPar unlimitedRange . d) (a . param)
  where
    unlimitedRange :: Param Range
    unlimitedRange = Param ["offset", "count"] $ \xs ->
      maybe (Left (ParseError "range"))
            (Right . normalize)
        $ case xs of
            [Just o, Just c] -> Range         <$> readMaybe o <*> readMaybe c
            [_     , Just c] -> Range 0       <$> readMaybe c
            [Just o, _     ] -> (`Range` 100) <$> readMaybe o
            _                -> Just $ Range 0 100
      where
        normalize r = Range
          { offset = max 0 . offset $ r
          , count  = max 0 . count $ r
          }

main :: IO ()
main = return ()