Skip to content
This repository has been archived by the owner on Apr 22, 2024. It is now read-only.

[WAIT] Issue 14: Add Request to ResponsePredicate #36

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 2 additions & 7 deletions src/Servant/QuickCheck/Internal/ErrorTypes.hs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import Prelude.Compat
import Text.PrettyPrint

data PredicateFailure
= PredicateFailure T.Text (Maybe C.Request) (C.Response LBS.ByteString)
= PredicateFailure T.Text (C.Request) (C.Response LBS.ByteString)
deriving (Typeable, Generic)

instance Exception ServerEqualityFailure where
Expand Down Expand Up @@ -71,10 +71,5 @@ prettyPredicateFailure :: PredicateFailure -> Doc
prettyPredicateFailure (PredicateFailure predicate req resp) =
text "Predicate failed" $$ (nest 5 $
text "Predicate:" <+> (text $ T.unpack predicate)
$$ r
$$ prettyReq req
$$ prettyResp resp)
where
r = case req of
Nothing -> text ""
Just v -> prettyReq v

54 changes: 28 additions & 26 deletions src/Servant/QuickCheck/Internal/Predicates.hs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,9 @@ import Servant.QuickCheck.Internal.ErrorTypes
--
-- /Since 0.0.0.0/
not500 :: ResponsePredicate
not500 = ResponsePredicate $ \resp ->
when (responseStatus resp == status500) $ throw $ PredicateFailure "not500" Nothing resp
not500 = ResponsePredicate $ \req resp ->
when (responseStatus resp == status500) $
throw $ PredicateFailure "not500" req resp

-- | [__Optional__]
--
Expand All @@ -58,7 +59,7 @@ notLongerThan maxAllowed
resp <- httpLbs req mgr
end <- getTime Monotonic
when (toNanoSecs (end `diffTimeSpec` start) > maxAllowed) $
throw $ PredicateFailure "notLongerThan" (Just req) resp
throw $ PredicateFailure "notLongerThan" req resp
return []

-- | [__Best Practice__]
Expand All @@ -84,8 +85,8 @@ notLongerThan maxAllowed
-- /Since 0.0.0.0/
onlyJsonObjects :: ResponsePredicate
onlyJsonObjects
= ResponsePredicate (\resp -> case go resp of
Nothing -> throw $ PredicateFailure "onlyJsonObjects" Nothing resp
= ResponsePredicate (\req resp -> case go resp of
Nothing -> throw $ PredicateFailure "onlyJsonObjects" req resp
Just () -> return ())
where
go r = do
Expand Down Expand Up @@ -120,12 +121,12 @@ createContainsValidLocation
resp <- httpLbs req mgr
if responseStatus resp == status201
then case lookup "Location" $ responseHeaders resp of
Nothing -> throw $ PredicateFailure n (Just req) resp
Nothing -> throw $ PredicateFailure n req resp
Just l -> case parseRequest $ SBSC.unpack l of
Nothing -> throw $ PredicateFailure n (Just req) resp
Nothing -> throw $ PredicateFailure n req resp
Just x -> do
resp2 <- httpLbs x mgr
status2XX (Just req) resp2 n
status2XX req resp2 n
return [resp, resp2]
else return [resp]

Expand Down Expand Up @@ -160,8 +161,8 @@ getsHaveLastModifiedHeader
if (method req == methodGet)
then do
resp <- httpLbs req mgr
unless (hasValidHeader "Last-Modified" isRFC822Date resp) $ do
throw $ PredicateFailure "getsHaveLastModifiedHeader" (Just req) resp
unless (hasValidHeader "Last-Modified" isRFC822Date resp) $
throw $ PredicateFailure "getsHaveLastModifiedHeader" req resp
return [resp]
else return []

Expand Down Expand Up @@ -193,7 +194,7 @@ notAllowedContainsAllowHeader
| m <- [minBound .. maxBound ]
, renderStdMethod m /= method req ]
case filter pred' resp of
(x:_) -> throw $ PredicateFailure "notAllowedContainsAllowHeader" (Just req) x
(x:_) -> throw $ PredicateFailure "notAllowedContainsAllowHeader" req x
[] -> return resp
where
pred' resp = responseStatus resp == status405 && not (hasValidHeader "Allow" go resp)
Expand Down Expand Up @@ -226,7 +227,7 @@ honoursAcceptHeader
sacc = fromMaybe "*/*" $ lookup "Accept" (requestHeaders req)
if status100 < scode && scode < status300
then if isJust $ sctype >>= \x -> matchAccept [x] sacc
then throw $ PredicateFailure "honoursAcceptHeader" (Just req) resp
then throw $ PredicateFailure "honoursAcceptHeader" req resp
else return [resp]
else return [resp]

Expand All @@ -251,8 +252,8 @@ getsHaveCacheControlHeader
if (method req == methodGet)
then do
resp <- httpLbs req mgr
unless (hasValidHeader "Cache-Control" (const True) resp) $ do
throw $ PredicateFailure "getsHaveCacheControlHeader" (Just req) resp
unless (hasValidHeader "Cache-Control" (const True) resp) $
throw $ PredicateFailure "getsHaveCacheControlHeader" req resp
return [resp]
else return []

Expand All @@ -268,7 +269,7 @@ headsHaveCacheControlHeader
then do
resp <- httpLbs req mgr
unless (hasValidHeader "Cache-Control" (const True) resp) $
throw $ PredicateFailure "headsHaveCacheControlHeader" (Just req) resp
throw $ PredicateFailure "headsHaveCacheControlHeader" req resp
return [resp]
else return []
{-
Expand Down Expand Up @@ -334,10 +335,10 @@ linkHeadersAreValid
-- /Since 0.0.0.0/
unauthorizedContainsWWWAuthenticate :: ResponsePredicate
unauthorizedContainsWWWAuthenticate
= ResponsePredicate $ \resp ->
= ResponsePredicate $ \req resp ->
if responseStatus resp == status401
then unless (hasValidHeader "WWW-Authenticate" (const True) resp) $
throw $ PredicateFailure "unauthorizedContainsWWWAuthenticate" Nothing resp
throw $ PredicateFailure "unauthorizedContainsWWWAuthenticate" req resp
else return ()


Expand All @@ -354,12 +355,12 @@ unauthorizedContainsWWWAuthenticate
-- /Since 0.3.0.0/
htmlIncludesDoctype :: ResponsePredicate
htmlIncludesDoctype
= ResponsePredicate $ \resp ->
= ResponsePredicate $ \req resp ->
if hasValidHeader "Content-Type" (SBS.isPrefixOf . foldCase $ "text/html") resp
then do
let htmlContent = foldCase . LBS.take 20 $ responseBody resp
unless (LBS.isPrefixOf (foldCase "<!doctype html>") htmlContent) $
throw $ PredicateFailure "htmlIncludesDoctype" Nothing resp
throw $ PredicateFailure "htmlIncludesDoctype" req resp
else return ()

-- * Predicate logic
Expand All @@ -374,12 +375,12 @@ htmlIncludesDoctype
--
-- /Since 0.0.0.0/
newtype ResponsePredicate = ResponsePredicate
{ getResponsePredicate :: Response LBS.ByteString -> IO ()
{ getResponsePredicate :: Request -> Response LBS.ByteString -> IO ()
} deriving (Generic)

instance Monoid ResponsePredicate where
mempty = ResponsePredicate $ const $ return ()
ResponsePredicate a `mappend` ResponsePredicate b = ResponsePredicate $ \x -> a x >> b x
mempty = ResponsePredicate (\req resp -> return ())
ResponsePredicate a `mappend` ResponsePredicate b = ResponsePredicate $ \x y -> a x y >> b x y

-- | A predicate that depends on both the request and the response.
--
Expand Down Expand Up @@ -429,7 +430,8 @@ finishPredicates p req mgr = go `catch` \(e :: PredicateFailure) -> return $ Jus
where
go = do
resps <- getRequestPredicate (requestPredicates p) req mgr
mapM_ (getResponsePredicate $ responsePredicates p) resps
let responder = getResponsePredicate (responsePredicates p) req
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, this isn't quite right, I think. The actual requests that resulted in the responses resps are not necessarily req. Rather, they are the requests generated by the RequestPredicates based on req.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to think a little more about this. It feels like it's becoming evident that the Request/ResponsePredicate distinction is somewhat artificial or misleading. Really what's at stake is something like the distinction between request generator (or generator modifiers) and (request + response) predicates. But separating those two too much is also wrong, since usually one wants to generate certain requests in order to check the responses.

Copy link
Contributor Author

@erewok erewok Oct 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am glad to have your perspective on this. I was wondering about the difference between ResponsePredicate and RequestPredicate when I was working on this PR, but I figured the distinction is still valid as a RequestPredicate needs a Manager in order to actually issue requests.

I have to confess I didn't look closely enough at finishPredicates, so it's a relief that you caught my mistake, which I can see clearly now.

I agree with what you're saying that checking a particular response is really often about a (Request, Response) pair. I was kind of errantly thinking about bifunctors or profunctors when looking at this code this weekend, but I didn't follow the thought very far.

mapM_ responder resps
return Nothing

-- * helpers
Expand All @@ -445,8 +447,8 @@ isRFC822Date s
Nothing -> False
Just (_ :: UTCTime) -> True

status2XX :: Monad m => Maybe Request -> Response LBS.ByteString -> T.Text -> m ()
status2XX mreq resp t
status2XX :: Monad m => Request -> Response LBS.ByteString -> T.Text -> m ()
status2XX req resp t
| status200 <= responseStatus resp && responseStatus resp < status300
= return ()
| otherwise = throw $ PredicateFailure t mreq resp
| otherwise = throw $ PredicateFailure t req resp
31 changes: 28 additions & 3 deletions test/Servant/QuickCheck/InternalSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -46,11 +46,13 @@ spec = do
serversEqualSpec
serverSatisfiesSpec
isComprehensiveSpec
no500s
onlyJsonObjectSpec
notLongerThanSpec
queryParamsSpec
queryFlagsSpec
deepPathSpec
authServerCheck
htmlDocTypesSpec
unbiasedGenerationSpec

Expand Down Expand Up @@ -127,6 +129,15 @@ serverSatisfiesSpec = describe "serverSatisfies" $ do
show err `shouldContain` "Body"


no500s :: Spec
no500s = describe "no500s" $ do

it "fails correctly" $ do
FailedWith err <- withServantServerAndContext api2 ctx server500fail $ \burl -> do
evalExample $ serverSatisfies api2 burl args
(not500 <%> mempty)
show err `shouldContain` "not500"

onlyJsonObjectSpec :: Spec
onlyJsonObjectSpec = describe "onlyJsonObjects" $ do

Expand Down Expand Up @@ -193,6 +204,17 @@ queryFlagsSpec = describe "QueryFlags" $ do
qs = C.unpack $ queryString req
qs `shouldBe` "one&two"

authServerCheck :: Spec
authServerCheck = describe "authenticate endpoints" $ do

it "authorization failure without WWWAuthenticate header fails correctly" $ do
FailedWith err <- withServantServerAndContext api2 ctx authFailServer $ \burl -> do
evalExample $ serverSatisfies api2 burl args
(unauthorizedContainsWWWAuthenticate <%> mempty)
show err `shouldContain` "unauthorizedContainsWWWAuthenticate"


-- Large API Randomness Testing Helper
htmlDocTypesSpec :: Spec
htmlDocTypesSpec = describe "HtmlDocTypes" $ do

Expand All @@ -217,7 +239,6 @@ makeRandomRequest large burl = do
req <- generate $ runGenRequest large
pure $ fst . fromJust . C.readInteger . C.drop 1 . path $ req burl


unbiasedGenerationSpec :: Spec
unbiasedGenerationSpec = describe "Unbiased Generation of requests" $

Expand Down Expand Up @@ -274,13 +295,18 @@ type DeepAPI = "one" :> "two" :> "three":> Get '[JSON] ()
deepAPI :: Proxy DeepAPI
deepAPI = Proxy


server2 :: IO (Server API2)
server2 = return $ return 1

server3 :: IO (Server API2)
server3 = return $ return 2

server500fail :: IO (Server API2)
server500fail = return $ throwError $ err500 { errBody = "BOOM!" }

authFailServer :: IO (Server API2)
authFailServer = return $ throwError $ err401 { errBody = "Login failure but missing header"}

-- With Doctypes
type HtmlDoctype = Get '[HTML] Blaze.Html

Expand All @@ -293,7 +319,6 @@ docTypeServer = pure $ pure $ Blaze5.docTypeHtml $ Blaze5.span "Hello Test!"
noDocTypeServer :: IO (Server HtmlDoctype)
noDocTypeServer = pure $ pure $ Blaze.text "Hello Test!"


-- Api for unbiased generation of requests tests
largeApi :: Proxy LargeAPI
largeApi = Proxy
Expand Down