From 8b34ad28409f9780fa49aa0a84d28a59ac46566a Mon Sep 17 00:00:00 2001 From: ksaric Date: Mon, 23 Jul 2018 16:04:18 +0200 Subject: [PATCH 01/23] [TSD-71] Generalize HTTP functions to a layer. --- log-classifier.cabal | 1 + src/DataSource.hs | 4 +- src/DataSource/Http.hs | 171 ++++++++++++++++++---------------------- src/DataSource/Types.hs | 51 ++++++++++-- src/HttpLayer.hs | 92 +++++++++++++++++++++ 5 files changed, 215 insertions(+), 104 deletions(-) create mode 100644 src/HttpLayer.hs diff --git a/log-classifier.cabal b/log-classifier.cabal index 770908e..af42a5b 100644 --- a/log-classifier.cabal +++ b/log-classifier.cabal @@ -31,6 +31,7 @@ library DataSource.DB DataSource.Types DataSource.Http + HttpLayer ghc-options: -Wall build-depends: aeson , array diff --git a/src/DataSource.hs b/src/DataSource.hs index da54e8c..7d24887 100644 --- a/src/DataSource.hs +++ b/src/DataSource.hs @@ -12,6 +12,7 @@ import Universum import DataSource.DB import DataSource.Http import DataSource.Types +import HttpLayer (basicHTTPNetworkLayer) -- | The default configuration. @@ -24,8 +25,9 @@ defaultConfig = Config , cfgAssignTo = 0 , cfgKnowledgebase = [] , cfgNumOfLogsToAnalyze = 5 - , cfgIsCommentPublic = True -- TODO(ks): For now, we need this in CLI. + , cfgIsCommentPublic = False -- TODO(ks): For now, we need this in CLI. , cfgZendeskLayer = basicZendeskLayer + , cfgHTTPNetworkLayer = basicHTTPNetworkLayer , cfgIOLayer = basicIOLayer , cfgDBLayer = connDBLayer } diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 1802c5c..da9e615 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -13,13 +13,12 @@ import Universum import Control.Concurrent (threadDelay) import Control.Monad.Reader (ask) -import Data.Aeson (FromJSON, ToJSON, Value, encode, parseJSON) +import Data.Aeson (parseJSON) import Data.Aeson.Text (encodeToLazyText) -import Data.Aeson.Types (Parser, parseEither) import Data.List (nub) -import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, httpLBS, - parseRequest_, setRequestBasicAuth, setRequestBodyJSON, - setRequestMethod, setRequestPath) +import Network.HTTP.Simple (Request, getResponseBody, httpLBS, parseRequest_) + +import HttpLayer import DataSource.Types (Attachment (..), AttachmentContent (..), Comment (..), CommentBody (..), CommentId (..), Config (..), @@ -27,10 +26,14 @@ import DataSource.Types (Attachment (..), AttachmentContent (..), Comm PageResultList (..), Ticket (..), TicketId (..), TicketInfo (..), TicketTag (..), TicketTags (..), User, UserId (..), ZendeskAPIUrl (..), ZendeskLayer (..), ZendeskResponse (..), - parseComments, parseTicket, renderTicketStatus, showURL) + asksHTTPNetworkLayer, parseComments, renderTicketStatus, showURL) -- ./mitmproxy --mode reverse:https://iohk.zendesk.com -p 4001 +------------------------------------------------------------ +-- Zendesk layer +------------------------------------------------------------ + -- | The basic Zendesk layer. -- The convention: -- - get returns a single result (wrapped in @Maybe@) @@ -65,6 +68,10 @@ emptyZendeskLayer = ZendeskLayer , zlExportTickets = \_ -> error "Not implemented zlExportTickets!" } +------------------------------------------------------------ +-- Zendesk functions +------------------------------------------------------------ + -- | Get single ticket info. getTicketInfo :: (MonadIO m, MonadReader Config m) @@ -75,7 +82,10 @@ getTicketInfo ticketId = do let url = showURL $ TicketsURL ticketId let req = apiRequest cfg url - liftIO $ Just <$> apiCall parseTicket req + + apiCall <- asksHTTPNetworkLayer hnlApiCall + + Just <$> apiCall parseJSON req -- | Return list of deleted tickets. listDeletedTickets @@ -146,23 +156,28 @@ getExportedTickets -> m [TicketInfo] getExportedTickets time = do cfg <- ask + + apiCall <- asksHTTPNetworkLayer hnlApiCall + let url = showURL $ ExportDataByTimestamp time let req = apiRequestAbsolute cfg url - iterateExportedTicketsWithDelay req + + iterateExportedTicketsWithDelay req (apiCall parseJSON) where iterateExportedTicketsWithDelay :: Request + -> (Request -> m (PageResultList TicketInfo)) -> m [TicketInfo] - iterateExportedTicketsWithDelay req = do + iterateExportedTicketsWithDelay req apiCall = do cfg <- ask - let go :: [TicketInfo] -> Text -> IO [TicketInfo] + let go :: [TicketInfo] -> Text -> m [TicketInfo] go list' nextPage' = do - threadDelay $ 10 * 1000000 -- Wait, Zendesk allows for 10 per minute. + liftIO $ threadDelay $ 10 * 1000000 -- Wait, Zendesk allows for 10 per minute. let req' = apiRequestAbsolute cfg nextPage' - (PageResultList pagen nextPagen count) <- apiCall parseJSON req' + (PageResultList pagen nextPagen count) <- apiCall req' case nextPagen of Just nextUrl -> if maybe False (>= 1000) count then go (list' <> pagen) nextUrl @@ -171,45 +186,11 @@ getExportedTickets time = do Nothing -> pure (list' <> pagen) - (PageResultList page0 nextPage _) <- liftIO $ apiCall parseJSON req + (PageResultList page0 nextPage _) <- apiCall req case nextPage of - Just nextUrl -> liftIO $ go page0 nextUrl + Just nextUrl -> go page0 nextUrl Nothing -> pure page0 --- | Iterate all the ticket pages and combine into a result. -iteratePages - :: forall m a. (MonadIO m, MonadReader Config m, FromPageResultList a) - => Request - -> m [a] -iteratePages req = iteratePagesWithDelay 0 req - --- | Iterate all the ticket pages and combine into a result. Wait for --- some time in-between the requests. -iteratePagesWithDelay - :: forall m a. (MonadIO m, MonadReader Config m, FromPageResultList a) - => Int - -> Request - -> m [a] -iteratePagesWithDelay seconds req = do - cfg <- ask - - let go :: [a] -> Text -> IO [a] - go list' nextPage' = do - -- Wait for @Int@ seconds. - threadDelay $ seconds * 1000000 - - let req' = apiRequestAbsolute cfg nextPage' - (PageResultList pagen nextPagen _) <- apiCall parseJSON req' - case nextPagen of - Just nextUrl -> go (list' <> pagen) nextUrl - Nothing -> pure (list' <> pagen) - - (PageResultList page0 nextPage _) <- liftIO $ apiCall parseJSON req - case nextPage of - Just nextUrl -> liftIO $ go page0 nextUrl - Nothing -> pure page0 - - -- | Send API request to post comment postTicketComment :: (MonadIO m, MonadReader Config m) @@ -218,10 +199,14 @@ postTicketComment -> m () postTicketComment ticketInfo zendeskResponse = do cfg <- ask + + addJsonBody <- asksHTTPNetworkLayer hnlAddJsonBody + apiCall <- asksHTTPNetworkLayer hnlApiCall + let responseTicket = createResponseTicket (cfgAgentId cfg) ticketInfo zendeskResponse let url = showURL $ TicketsURL (zrTicketId zendeskResponse) let req = addJsonBody responseTicket (apiRequest cfg url) - void $ liftIO $ apiCall (pure . encodeToLazyText) req + void $ apiCall (pure . encodeToLazyText) req -- | Create response ticket createResponseTicket :: Integer -> TicketInfo -> ZendeskResponse -> Ticket @@ -248,10 +233,12 @@ _getUser _getUser = do cfg <- ask + apiCall <- asksHTTPNetworkLayer hnlApiCall + let url = showURL UserInfoURL let req = apiRequest cfg url - liftIO $ apiCall parseJSON req + apiCall parseJSON req -- | Given attachmentUrl, return attachment in bytestring getAttachment @@ -271,10 +258,12 @@ getTicketComments getTicketComments tId = do cfg <- ask + apiCallSafe <- asksHTTPNetworkLayer hnlApiCallSafe + let url = showURL $ TicketCommentsURL tId let req = apiRequest cfg url - result <- apiCallSafe parseComments req + result <- apiCallSafe parseComments req -- TODO(ks): For now return empty if there is an exception. -- After we have exception handling, we propagate this up. @@ -283,51 +272,41 @@ getTicketComments tId = do Right r -> pure r ------------------------------------------------------------ --- HTTP utility +-- Utility ------------------------------------------------------------ --- | Request PUT -addJsonBody :: ToJSON a => a -> Request -> Request -addJsonBody body req = setRequestBodyJSON body $ setRequestMethod "PUT" req - --- | Make an api call --- TODO(ks): Switch to @Either@. -apiCall :: FromJSON a => (Value -> Parser a) -> Request -> IO a -apiCall parser req = do - putTextLn $ show req - v <- getResponseBody <$> httpJSON req - case parseEither parser v of - Right o -> pure o - Left e -> error $ "couldn't parse response " - <> toText e <> "\n" <> decodeUtf8 (encode v) - --- | Make a safe api call. -apiCallSafe - :: forall m a. (MonadIO m, FromJSON a) - => (Value -> Parser a) +-- | Iterate all the ticket pages and combine into a result. +iteratePages + :: forall m a. (MonadIO m, MonadReader Config m, FromPageResultList a) + => Request + -> m [a] +iteratePages req = iteratePagesWithDelay 0 req + +-- | Iterate all the ticket pages and combine into a result. Wait for +-- some time in-between the requests. +iteratePagesWithDelay + :: forall m a. (MonadIO m, MonadReader Config m, FromPageResultList a) + => Int -> Request - -> m (Either String a) -apiCallSafe parser req = do - putTextLn $ show req - v <- getResponseBody <$> httpJSON req - pure $ parseEither parser v - --- | General api request function -apiRequest :: Config -> Text -> Request -apiRequest Config{..} u = setRequestPath (encodeUtf8 path) $ - addRequestHeader "Content-Type" "application/json" $ - setRequestBasicAuth - (encodeUtf8 cfgEmail <> "/token") - (encodeUtf8 cfgToken) $ - parseRequest_ (toString (cfgZendesk <> path)) - where - path :: Text - path = "/api/v2" <> u - --- | Api request but use absolute path -apiRequestAbsolute :: Config -> Text -> Request -apiRequestAbsolute Config{..} u = addRequestHeader "Content-Type" "application/json" $ - setRequestBasicAuth - (encodeUtf8 cfgEmail <> "/token") - (encodeUtf8 cfgToken) $ - parseRequest_ (toString u) + -> m [a] +iteratePagesWithDelay seconds req = do + cfg <- ask + + apiCall <- asksHTTPNetworkLayer hnlApiCall + + let go :: [a] -> Text -> IO [a] + go list' nextPage' = do + -- Wait for @Int@ seconds. + threadDelay $ seconds * 1000000 + + let req' = apiRequestAbsolute cfg nextPage' + (PageResultList pagen nextPagen _) <- apiCall parseJSON req' + case nextPagen of + Just nextUrl -> go (list' <> pagen) nextUrl + Nothing -> pure (list' <> pagen) + + (PageResultList page0 nextPage _) <- liftIO $ apiCall parseJSON req + case nextPage of + Just nextUrl -> liftIO $ go page0 nextUrl + Nothing -> pure page0 + diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index d427d46..3ce203f 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -1,6 +1,7 @@ {-# LANGUAGE DeriveGeneric #-} {-# LANGUAGE GeneralizedNewtypeDeriving #-} {-# LANGUAGE OverloadedStrings #-} +{-# LANGUAGE RankNTypes #-} {-# LANGUAGE RecordWildCards #-} module DataSource.Types @@ -40,12 +41,14 @@ module DataSource.Types , App , Config (..) , ZendeskLayer (..) + , HTTPNetworkLayer (..) , IOLayer (..) , DBLayer (..) , knowledgebasePath , tokenPath , assignToPath , asksZendeskLayer + , asksHTTPNetworkLayer , asksIOLayer , asksDBLayer , showURL @@ -54,6 +57,9 @@ module DataSource.Types import Universum +import Control.Monad.Base (MonadBase) +import Control.Monad.Trans.Control (MonadBaseControl (..)) + import UnliftIO (MonadUnliftIO) import Data.Aeson (FromJSON, ToJSON, Value (Object), object, parseJSON, toJSON, @@ -61,10 +67,9 @@ import Data.Aeson (FromJSON, ToJSON, Value (Object), object, parseJSON import Data.Aeson.Types (Parser) import qualified Data.Text as T import Data.Time.Clock.POSIX (POSIXTime) -import LogAnalysis.Types (Knowledge) +import Network.HTTP.Simple (Request) -import Control.Monad.Base (MonadBase) -import Control.Monad.Trans.Control (MonadBaseControl (..)) +import LogAnalysis.Types (Knowledge) import Test.QuickCheck (Arbitrary (..), elements, listOf1, vectorOf) @@ -118,25 +123,44 @@ data Config = Config , cfgDBLayer :: !(DBLayer App) -- ^ The _DB_ layer. This is containing all the modification functions. -- TODO(ks): @Maybe@ db layer. It's not really required. + , cfgHTTPNetworkLayer :: !HTTPNetworkLayer + -- ^ The HTTP network layer. Required so we can mock out the responses and separate + -- the specific HTTP communication (library) from the code that uses it. } -- | Utility function for getting a function of the @ZendeskLayer@. -asksZendeskLayer :: forall m a. (MonadReader Config m) => (ZendeskLayer App -> a) -> m a +asksZendeskLayer + :: forall m a. (MonadReader Config m) + => (ZendeskLayer App -> a) + -> m a asksZendeskLayer getter = do Config{..} <- ask pure $ getter cfgZendeskLayer +-- | Utility function for getting a function of the @cfgHTTPNetworkLayer@. +asksHTTPNetworkLayer + :: forall m a. (MonadReader Config m) + => (HTTPNetworkLayer -> a) + -> m a +asksHTTPNetworkLayer getter = do + Config{..} <- ask + pure $ getter cfgHTTPNetworkLayer -- | Utility function for getting a function of the @ZendeskLayer@. -asksIOLayer :: forall m a. (MonadReader Config m) => (IOLayer App -> a) -> m a +asksIOLayer + :: forall m a. (MonadReader Config m) + => (IOLayer App -> a) + -> m a asksIOLayer getter = do Config{..} <- ask pure $ getter cfgIOLayer - -- | Utility function for getting a function of the @DBLayer@. -asksDBLayer :: forall m a. (MonadReader Config m) => (DBLayer App -> a) -> m a +asksDBLayer + :: forall m a. (MonadReader Config m) + => (DBLayer App -> a) + -> m a asksDBLayer getter = do Config{..} <- ask pure $ getter cfgDBLayer @@ -174,6 +198,19 @@ data ZendeskLayer m = ZendeskLayer , zlExportTickets :: ExportFromTime -> m [TicketInfo] } +-- | The HTTP network layer that we want to expose. +-- We don't want anything to leak out, so we expose only the most relevant information, +-- anything relating to how it internaly works should NOT be exposed. +-- Actually, it would be better if we "inject" this into +-- the @Http@ module, say in the @ZendeskaLayer@, but it's good enough for now. +-- We need to use RankNTypes due to some complications that appeared. +data HTTPNetworkLayer = HTTPNetworkLayer + { hnlAddJsonBody :: forall a. (ToJSON a) => a -> Request -> Request + -- TODO(ks): These two below are basically the same, will fix in future PR, requires refactoring. + , hnlApiCall :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m a + , hnlApiCallSafe :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) + } + -- | The IOLayer interface that we can expose. -- We want to do this since we want to be able to mock out any function tied to @IO@. data IOLayer m = IOLayer diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs new file mode 100644 index 0000000..fc32350 --- /dev/null +++ b/src/HttpLayer.hs @@ -0,0 +1,92 @@ +module HttpLayer + ( HTTPNetworkLayer (..) + , basicHTTPNetworkLayer + , emptyHTTPNetworkLayer + , apiRequest + , apiRequestAbsolute + ) where + +import Universum + +import Data.Aeson (FromJSON, ToJSON, Value, encode) +import Data.Aeson.Types (Parser, parseEither) +import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, + parseRequest_, setRequestBasicAuth, setRequestBodyJSON, + setRequestMethod, setRequestPath) + +import DataSource.Types (Config (..), HTTPNetworkLayer (..)) + + +------------------------------------------------------------ +-- Layer +------------------------------------------------------------ + +basicHTTPNetworkLayer :: HTTPNetworkLayer +basicHTTPNetworkLayer = HTTPNetworkLayer + { hnlAddJsonBody = addJsonBody + , hnlApiCall = apiCall + , hnlApiCallSafe = apiCallSafe + } + +emptyHTTPNetworkLayer :: HTTPNetworkLayer +emptyHTTPNetworkLayer = HTTPNetworkLayer + { hnlAddJsonBody = \_ _ -> error "hnlAddJsonBody not implemented!" + , hnlApiCall = \_ -> error "hnlApiCall not implemented!" + , hnlApiCallSafe = \_ _ -> error "hnlApiCallSafe not implemented!" + } + +------------------------------------------------------------ +-- Functions +------------------------------------------------------------ + +-- | General api request function +apiRequest :: Config -> Text -> Request +apiRequest Config{..} u = + setRequestPath (encodeUtf8 path) $ + addRequestHeader "Content-Type" "application/json" $ + setRequestBasicAuth + (encodeUtf8 cfgEmail <> "/token") + (encodeUtf8 cfgToken) $ + parseRequest_ (toString (cfgZendesk <> path)) + where + path :: Text + path = "/api/v2" <> u + +-- | Api request but use absolute path +apiRequestAbsolute :: Config -> Text -> Request +apiRequestAbsolute Config{..} u = + addRequestHeader "Content-Type" "application/json" $ + setRequestBasicAuth (encodeUtf8 cfgEmail <> "/token") (encodeUtf8 cfgToken) $ + parseRequest_(toString u) + +-- | Request PUT +addJsonBody :: forall a. ToJSON a => a -> Request -> Request +addJsonBody body req = setRequestBodyJSON body $ setRequestMethod "PUT" req + +-- | Make an api call +-- TODO(ks): Switch to @Either@. +apiCall + :: forall m a. (MonadIO m, FromJSON a) + => (Value -> Parser a) + -> Request + -> m a +apiCall parser req = do + putTextLn $ show req + v <- getResponseBody <$> httpJSON req + case parseEither parser v of + Right o -> pure o + Left e -> error $ "couldn't parse response " + <> toText e <> "\n" <> decodeUtf8 (encode v) + +-- | Make a safe api call. +apiCallSafe + :: forall m a. (MonadIO m, FromJSON a) + => (Value -> Parser a) + -> Request + -> m (Either String a) +apiCallSafe parser req = do + putTextLn $ show req + v <- getResponseBody <$> httpJSON req + pure $ parseEither parser v + + From 1265e0e0177ccfae72e417d77e1751f5f52e3e3b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 24 Jul 2018 16:16:30 +0200 Subject: [PATCH 02/23] Adds an Aeson-related empty exception --- src/Exceptions.hs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/Exceptions.hs b/src/Exceptions.hs index 0dc7dca..0e55ba9 100644 --- a/src/Exceptions.hs +++ b/src/Exceptions.hs @@ -27,13 +27,21 @@ data ZipFileExceptions -- ^ Decompresson of a zip file was not sucessful deriving Show +-- | Exceptions that occur during JSON parsing +data JSONParsingException + = MkJSONParsingException + instance Exception ProcessTicketExceptions instance Exception ZipFileExceptions +instance Exception JSONParsingException instance Show ProcessTicketExceptions where show (AttachmentNotFound tid) = "Attachment was not found on ticket ID: " <> showTicketId tid show (CommentAndAttachmentNotFound tid) = "Both comment and attachment were not found on ticket ID: " <> showTicketId tid show (TicketInfoNotFound tid) = "Ticket information was not found on ticket ID: " <> showTicketId tid +instance Show JSONParsingException where + show MkJSONParsingException = "" + showTicketId :: TicketId -> String showTicketId tid = Universum.show (getTicketId tid) From 6591ffd4ea881bf4a3b78597d16290c37e10759b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 25 Jul 2018 16:11:41 +0200 Subject: [PATCH 03/23] WIP: adds support for handling JSON parsing exceptions --- src/DataSource/Http.hs | 23 +++++++++++++---------- src/DataSource/Types.hs | 2 +- src/Exceptions.hs | 1 + 3 files changed, 15 insertions(+), 11 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index da9e615..2663651 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -39,7 +39,7 @@ import DataSource.Types (Attachment (..), AttachmentContent (..), Comm -- - get returns a single result (wrapped in @Maybe@) -- - list returns multiple results -- - post submits a result (maybe PUT?!) -basicZendeskLayer :: (MonadIO m, MonadReader Config m) => ZendeskLayer m +basicZendeskLayer :: (MonadIO m, MonadReader Config m, MonadCatch m) => ZendeskLayer m basicZendeskLayer = ZendeskLayer { zlGetTicketInfo = getTicketInfo , zlListDeletedTickets = listDeletedTickets @@ -74,18 +74,22 @@ emptyZendeskLayer = ZendeskLayer -- | Get single ticket info. getTicketInfo - :: (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadCatch m) => TicketId - -> m (Maybe TicketInfo) -getTicketInfo ticketId = do - cfg <- ask + -> m TicketInfo +getTicketInfo ticketId = + catch getInfo $ \(e) -> throwM e -- TODO(md): Change the exception type to JSONParsingException + where + getInfo :: m TicketInfo + getInfo = do + cfg <- ask - let url = showURL $ TicketsURL ticketId - let req = apiRequest cfg url + let url = showURL $ TicketsURL ticketId + let req = apiRequest cfg url - apiCall <- asksHTTPNetworkLayer hnlApiCall + apiCall <- asksHTTPNetworkLayer hnlApiCall - Just <$> apiCall parseJSON req + apiCall parseJSON req -- | Return list of deleted tickets. listDeletedTickets @@ -309,4 +313,3 @@ iteratePagesWithDelay seconds req = do case nextPage of Just nextUrl -> liftIO $ go page0 nextUrl Nothing -> pure page0 - diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index 3ce203f..e52bde9 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -184,7 +184,7 @@ assignToPath = "./tmp-secrets/assign_to" -- We don't want anything to leak out, so we expose only the most relevant information, -- anything relating to how it internaly works should NOT be exposed. data ZendeskLayer m = ZendeskLayer - { zlGetTicketInfo :: TicketId -> m (Maybe TicketInfo) + { zlGetTicketInfo :: TicketId -> m TicketInfo , zlListDeletedTickets :: m [DeletedTicket] , zlListRequestedTickets :: UserId -> m [TicketInfo] , zlListAssignedTickets :: UserId -> m [TicketInfo] diff --git a/src/Exceptions.hs b/src/Exceptions.hs index 0e55ba9..540a2d1 100644 --- a/src/Exceptions.hs +++ b/src/Exceptions.hs @@ -1,6 +1,7 @@ module Exceptions ( ProcessTicketExceptions (..) , ZipFileExceptions (..) + , JSONParsingException (..) ) where import Universum From 8b56390de50ea35bca8d97626fe8ed9fdf5ff093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Thu, 26 Jul 2018 16:03:54 +0200 Subject: [PATCH 04/23] WIP: Fixes some compilation errors wrt exceptions, introduces some new --- src/DataSource/Http.hs | 4 ++-- src/DataSource/Types.hs | 2 +- src/Exceptions.hs | 9 --------- src/HttpLayer.hs | 21 ++++++++++++++++----- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 2663651..1ecf939 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -19,7 +19,7 @@ import Data.List (nub) import Network.HTTP.Simple (Request, getResponseBody, httpLBS, parseRequest_) import HttpLayer - +import HttpLayer (JSONParsingException (..)) import DataSource.Types (Attachment (..), AttachmentContent (..), Comment (..), CommentBody (..), CommentId (..), Config (..), DeletedTicket (..), ExportFromTime (..), FromPageResultList (..), @@ -78,7 +78,7 @@ getTicketInfo => TicketId -> m TicketInfo getTicketInfo ticketId = - catch getInfo $ \(e) -> throwM e -- TODO(md): Change the exception type to JSONParsingException + catch getInfo $ \_ -> throwM $ MkJSONParsingException "a" -- TODO(md): Change the exception type to JSONParsingException where getInfo :: m TicketInfo getInfo = do diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index e52bde9..809b878 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -207,7 +207,7 @@ data ZendeskLayer m = ZendeskLayer data HTTPNetworkLayer = HTTPNetworkLayer { hnlAddJsonBody :: forall a. (ToJSON a) => a -> Request -> Request -- TODO(ks): These two below are basically the same, will fix in future PR, requires refactoring. - , hnlApiCall :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m a + , hnlApiCall :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request -> m a , hnlApiCallSafe :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) } diff --git a/src/Exceptions.hs b/src/Exceptions.hs index 540a2d1..0dc7dca 100644 --- a/src/Exceptions.hs +++ b/src/Exceptions.hs @@ -1,7 +1,6 @@ module Exceptions ( ProcessTicketExceptions (..) , ZipFileExceptions (..) - , JSONParsingException (..) ) where import Universum @@ -28,21 +27,13 @@ data ZipFileExceptions -- ^ Decompresson of a zip file was not sucessful deriving Show --- | Exceptions that occur during JSON parsing -data JSONParsingException - = MkJSONParsingException - instance Exception ProcessTicketExceptions instance Exception ZipFileExceptions -instance Exception JSONParsingException instance Show ProcessTicketExceptions where show (AttachmentNotFound tid) = "Attachment was not found on ticket ID: " <> showTicketId tid show (CommentAndAttachmentNotFound tid) = "Both comment and attachment were not found on ticket ID: " <> showTicketId tid show (TicketInfoNotFound tid) = "Ticket information was not found on ticket ID: " <> showTicketId tid -instance Show JSONParsingException where - show MkJSONParsingException = "" - showTicketId :: TicketId -> String showTicketId tid = Universum.show (getTicketId tid) diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index fc32350..dadfa8a 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -1,5 +1,6 @@ module HttpLayer ( HTTPNetworkLayer (..) + , JSONParsingException (..) , basicHTTPNetworkLayer , emptyHTTPNetworkLayer , apiRequest @@ -10,12 +11,24 @@ import Universum import Data.Aeson (FromJSON, ToJSON, Value, encode) import Data.Aeson.Types (Parser, parseEither) +import Data.Text (unpack) import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, parseRequest_, setRequestBasicAuth, setRequestBodyJSON, setRequestMethod, setRequestPath) import DataSource.Types (Config (..), HTTPNetworkLayer (..)) +import qualified Prelude (Show(..)) +------------------------------------------------------------ +-- JSON parsing exceptions +------------------------------------------------------------ +-- | Exceptions that occur during JSON parsing +data JSONParsingException + = MkJSONParsingException !Text + +instance Exception JSONParsingException +instance Prelude.Show JSONParsingException where + show (MkJSONParsingException s) = "JSON parsing exception: " <> (unpack s) ------------------------------------------------------------ -- Layer @@ -66,7 +79,7 @@ addJsonBody body req = setRequestBodyJSON body $ setRequestMethod "PUT" req -- | Make an api call -- TODO(ks): Switch to @Either@. apiCall - :: forall m a. (MonadIO m, FromJSON a) + :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request -> m a @@ -75,8 +88,8 @@ apiCall parser req = do v <- getResponseBody <$> httpJSON req case parseEither parser v of Right o -> pure o - Left e -> error $ "couldn't parse response " - <> toText e <> "\n" <> decodeUtf8 (encode v) + Left e -> throwM $ MkJSONParsingException $ + "couldn't parse response " <> toText e <> "\n" <> decodeUtf8 (encode v) -- | Make a safe api call. apiCallSafe @@ -88,5 +101,3 @@ apiCallSafe parser req = do putTextLn $ show req v <- getResponseBody <$> httpJSON req pure $ parseEither parser v - - From b846064bd95ffdfe2bfb491225fa66b68bd668ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 27 Jul 2018 14:07:03 +0200 Subject: [PATCH 05/23] Resolves a few compile errors by adding the MonadThrow constraint --- src/DataSource/Http.hs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 1d95342..166d824 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -78,7 +78,7 @@ getTicketInfo => TicketId -> m TicketInfo getTicketInfo ticketId = - catch getInfo $ \_ -> throwM $ MkJSONParsingException "a" -- TODO(md): Change the exception type to JSONParsingException + catch getInfo $ \(e :: JSONParsingException) -> throwM e -- TODO(md): Change the exception type to JSONParsingException where getInfo :: m TicketInfo getInfo = do @@ -155,7 +155,7 @@ listAdminAgents = do -- NOTE: If count is less than 1000, then stop paginating. -- Otherwise, use the next_page URL to get the next page of results. getExportedTickets - :: forall m. (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => ExportFromTime -> m [TicketInfo] getExportedTickets time = do @@ -197,7 +197,7 @@ getExportedTickets time = do -- | Send API request to post comment postTicketComment - :: (MonadIO m, MonadReader Config m) + :: (MonadIO m, MonadReader Config m, MonadThrow m) => TicketInfo -> ZendeskResponse -> m () @@ -232,7 +232,7 @@ createResponseTicket agentId TicketInfo{..} ZendeskResponse{..} = -- | Get user information. _getUser - :: (MonadIO m, MonadReader Config m) + :: (MonadIO m, MonadReader Config m, MonadThrow m) => m User _getUser = do cfg <- ask From 5ffc1867e105a964e613d785559b9db46644feab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 27 Jul 2018 14:58:10 +0200 Subject: [PATCH 06/23] Fixes a few Maybe TicketInfo compilation errors --- src/DataSource/DB.hs | 1 - src/DataSource/Http.hs | 6 +++--- src/DataSource/Types.hs | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/DataSource/DB.hs b/src/DataSource/DB.hs index 525c2f8..2f8efff 100644 --- a/src/DataSource/DB.hs +++ b/src/DataSource/DB.hs @@ -448,4 +448,3 @@ deleteAllData conn = do deleteCommentAttachments conn deleteTicketComments conn deleteTickets conn - diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 166d824..bc8f17e 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -76,11 +76,11 @@ emptyDataLayer = DataLayer getTicketInfo :: forall m. (MonadIO m, MonadReader Config m, MonadCatch m) => TicketId - -> m TicketInfo + -> m (Maybe TicketInfo) getTicketInfo ticketId = catch getInfo $ \(e :: JSONParsingException) -> throwM e -- TODO(md): Change the exception type to JSONParsingException where - getInfo :: m TicketInfo + getInfo :: m (Maybe TicketInfo) getInfo = do cfg <- ask @@ -89,7 +89,7 @@ getTicketInfo ticketId = apiCall <- asksHTTPNetworkLayer hnlApiCall - apiCall parseJSON req + Just <$> apiCall parseJSON req -- | Return list of deleted tickets. listDeletedTickets diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index abca705..a410703 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -184,7 +184,7 @@ assignToPath = "./tmp-secrets/assign_to" -- We don't want anything to leak out, so we expose only the most relevant information, -- anything relating to how it internaly works should NOT be exposed. data DataLayer m = DataLayer - { zlGetTicketInfo :: TicketId -> m TicketInfo + { zlGetTicketInfo :: TicketId -> m (Maybe TicketInfo) , zlListDeletedTickets :: m [DeletedTicket] , zlListRequestedTickets :: UserId -> m [TicketInfo] , zlListAssignedTickets :: UserId -> m [TicketInfo] From 404322cbaa6b85f0ae284071ca6b45f636dc7d07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 27 Jul 2018 15:01:01 +0200 Subject: [PATCH 07/23] Fixes remaining compilation errors (MonadCatch constraint) --- src/DataSource/DB.hs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/DataSource/DB.hs b/src/DataSource/DB.hs index 2f8efff..52d9730 100644 --- a/src/DataSource/DB.hs +++ b/src/DataSource/DB.hs @@ -113,7 +113,7 @@ emptyDBLayer = DBLayer -- | The simple connection Zendesk layer. Used for database querying. -- We need to sync occasionaly. -connDataLayer :: forall m. (MonadIO m, MonadReader Config m) => DataLayer m +connDataLayer :: forall m. (MonadIO m, MonadReader Config m, MonadCatch m) => DataLayer m connDataLayer = DataLayer { zlGetTicketInfo = \tId -> withProdDatabase $ \conn -> getTicketInfoByTicketId conn tId , zlListDeletedTickets = zlListDeletedTickets basicDataLayer @@ -147,7 +147,7 @@ connDBLayer = DBLayer -- | The connection pooled Zendesk layer. Used for database querying. -- We need to sync occasionaly. -connPoolDataLayer :: forall m. (MonadBaseControl IO m, MonadIO m, MonadReader Config m) => DBConnPool -> DataLayer m +connPoolDataLayer :: forall m. (MonadBaseControl IO m, MonadIO m, MonadReader Config m, MonadCatch m) => DBConnPool -> DataLayer m connPoolDataLayer connPool = DataLayer { zlGetTicketInfo = \tId -> withConnPool connPool $ \conn -> getTicketInfoByTicketId conn tId , zlListDeletedTickets = zlListDeletedTickets basicDataLayer From 7845bcda138e0764fe369e5d35020b7cb226d97d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Fri, 27 Jul 2018 15:06:54 +0200 Subject: [PATCH 08/23] Minor polishing (data -> newtype) --- src/DataSource/Http.hs | 2 +- src/HttpLayer.hs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index bc8f17e..1f42ef7 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -78,7 +78,7 @@ getTicketInfo => TicketId -> m (Maybe TicketInfo) getTicketInfo ticketId = - catch getInfo $ \(e :: JSONParsingException) -> throwM e -- TODO(md): Change the exception type to JSONParsingException + catch getInfo $ \(e :: JSONParsingException) -> throwM e where getInfo :: m (Maybe TicketInfo) getInfo = do diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index dadfa8a..4b106ff 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -23,8 +23,7 @@ import qualified Prelude (Show(..)) -- JSON parsing exceptions ------------------------------------------------------------ -- | Exceptions that occur during JSON parsing -data JSONParsingException - = MkJSONParsingException !Text +newtype JSONParsingException = MkJSONParsingException Text instance Exception JSONParsingException instance Prelude.Show JSONParsingException where From 3b0e885c4f10de93d7c596949d40a76715de9ab7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 31 Jul 2018 13:40:16 +0200 Subject: [PATCH 09/23] Changes the type of hnlAddJsonBody --- cabal2nix.nix | 2 +- log-classifier.cabal | 1 + src/DataSource/Types.hs | 2 +- src/HttpLayer.hs | 9 +++++++-- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/cabal2nix.nix b/cabal2nix.nix index 7d0e93f..a453a9e 100644 --- a/cabal2nix.nix +++ b/cabal2nix.nix @@ -1,5 +1,5 @@ { mkDerivation, aeson, array, attoparsec, base, bytestring -, containers, directory, generics-sop, hspec, http-conduit +, containers, directory, either, generics-sop, hspec, http-conduit , monad-control, mtl, optparse-applicative, QuickCheck, reflection , regex-tdfa, resource-pool, sqlite-simple, stdenv, text, time , transformers-base, universum, unliftio, zip-archive diff --git a/log-classifier.cabal b/log-classifier.cabal index 2b8c148..a1519ab 100644 --- a/log-classifier.cabal +++ b/log-classifier.cabal @@ -41,6 +41,7 @@ library , bytestring , containers , directory + , either , http-conduit -- Effects & company , mtl diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index 19a7a11..8bd0d73 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -204,7 +204,7 @@ data DataLayer m = DataLayer -- the @Http@ module, say in the @ZendeskaLayer@, but it's good enough for now. -- We need to use RankNTypes due to some complications that appeared. data HTTPNetworkLayer = HTTPNetworkLayer - { hnlAddJsonBody :: forall a. (ToJSON a) => a -> Request -> Request + { hnlAddJsonBody :: forall a. (ToJSON a) => a -> Request -> Either String Request -- TODO(ks): These two below are basically the same, will fix in future PR, requires refactoring. , hnlApiCall :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request -> m a , hnlApiCallSafe :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index 4b106ff..0edeb9b 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -11,6 +11,7 @@ import Universum import Data.Aeson (FromJSON, ToJSON, Value, encode) import Data.Aeson.Types (Parser, parseEither) +import Data.Either.Combinators (mapLeft) import Data.Text (unpack) import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, parseRequest_, setRequestBasicAuth, setRequestBodyJSON, @@ -72,8 +73,12 @@ apiRequestAbsolute Config{..} u = parseRequest_(toString u) -- | Request PUT -addJsonBody :: forall a. ToJSON a => a -> Request -> Request -addJsonBody body req = setRequestBodyJSON body $ setRequestMethod "PUT" req +addJsonBody :: forall a. ToJSON a => a -> Request -> Either String Request +addJsonBody body req = mapLeft show $ catch updateReq Left + where + updateReq :: Exception e => Either e Request + updateReq = Right <$> setRequestBodyJSON body $ setRequestMethod "PUT" req + -- | Make an api call -- TODO(ks): Switch to @Either@. From d6745bec7eaf7112ecda1f9c8d523d746e418aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 1 Aug 2018 12:09:31 +0200 Subject: [PATCH 10/23] WIP: Changed the type of the apiRequest function; doesn't compile at the moment --- src/DataSource/Http.hs | 17 +++++++++------ src/DataSource/Types.hs | 2 +- src/HttpLayer.hs | 46 +++++++++++++++++++++++------------------ 3 files changed, 38 insertions(+), 27 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 195482f..22aea96 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -16,10 +16,11 @@ import Control.Monad.Reader (ask) import Data.Aeson (parseJSON) import Data.Aeson.Text (encodeToLazyText) import Data.List (nub) +import Data.Text (pack) import Network.HTTP.Simple (Request, getResponseBody, httpLBS, parseRequest_) import HttpLayer (HTTPNetworkLayer (..), apiRequest, apiRequestAbsolute, - JSONParsingException (..)) + JSONException (..)) import DataSource.Types (Attachment (..), AttachmentContent (..), Comment (..), CommentBody (..), CommentId (..), Config (..), DataLayer (..), @@ -79,7 +80,7 @@ getTicketInfo => TicketId -> m (Maybe TicketInfo) getTicketInfo ticketId = - catch getInfo $ \(e :: JSONParsingException) -> throwM e + catch getInfo $ \(e :: JSONException) -> throwM e where getInfo :: m (Maybe TicketInfo) getInfo = do @@ -90,7 +91,7 @@ getTicketInfo ticketId = apiCall <- asksHTTPNetworkLayer hnlApiCall - Just <$> apiCall parseTicket req + rightToMaybe <$> apiCall parseTicket req -- | Return list of deleted tickets. listDeletedTickets @@ -211,7 +212,10 @@ postTicketComment ticketInfo zendeskResponse = do let responseTicket = createResponseTicket (cfgAgentId cfg) ticketInfo zendeskResponse let url = showURL $ TicketsURL (zrTicketId zendeskResponse) let req = addJsonBody responseTicket (apiRequest cfg url) - void $ apiCall (pure . encodeToLazyText) req + case req of + Left e -> throwM $ JSONEncodingException $ pack e + Right r -> void $ apiCall (pure . encodeToLazyText) r + -- | Create response ticket createResponseTicket :: Integer -> TicketInfo -> ZendeskResponse -> Ticket @@ -242,8 +246,9 @@ _getUser = do let url = showURL UserInfoURL let req = apiRequest cfg url - - apiCall parseJSON req + case req of + Left e -> throwM $ JSONParsingException e + Right r -> apiCall parseJSON r -- | Given attachmentUrl, return attachment in bytestring getAttachment diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index 8bd0d73..efc273b 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -206,7 +206,7 @@ data DataLayer m = DataLayer data HTTPNetworkLayer = HTTPNetworkLayer { hnlAddJsonBody :: forall a. (ToJSON a) => a -> Request -> Either String Request -- TODO(ks): These two below are basically the same, will fix in future PR, requires refactoring. - , hnlApiCall :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request -> m a + , hnlApiCall :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) , hnlApiCallSafe :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) } diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index 0edeb9b..cf1cee6 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -1,6 +1,6 @@ module HttpLayer ( HTTPNetworkLayer (..) - , JSONParsingException (..) + , JSONException (..) , basicHTTPNetworkLayer , emptyHTTPNetworkLayer , apiRequest @@ -15,7 +15,7 @@ import Data.Either.Combinators (mapLeft) import Data.Text (unpack) import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, parseRequest_, setRequestBasicAuth, setRequestBodyJSON, - setRequestMethod, setRequestPath) + setRequestMethod, setRequestPath, parseRequest) import DataSource.Types (Config (..), HTTPNetworkLayer (..)) import qualified Prelude (Show(..)) @@ -24,11 +24,14 @@ import qualified Prelude (Show(..)) -- JSON parsing exceptions ------------------------------------------------------------ -- | Exceptions that occur during JSON parsing -newtype JSONParsingException = MkJSONParsingException Text +data JSONException + = JSONParsingException Text + | JSONEncodingException Text -instance Exception JSONParsingException -instance Prelude.Show JSONParsingException where - show (MkJSONParsingException s) = "JSON parsing exception: " <> (unpack s) +instance Exception JSONException +instance Prelude.Show JSONException where + show (JSONParsingException s) = "JSON parsing exception: " <> (unpack s) + show (JSONEncodingException s) = "JSON encoding exception: " <> (unpack s) ------------------------------------------------------------ -- Layer @@ -53,15 +56,17 @@ emptyHTTPNetworkLayer = HTTPNetworkLayer ------------------------------------------------------------ -- | General api request function -apiRequest :: Config -> Text -> Request -apiRequest Config{..} u = - setRequestPath (encodeUtf8 path) $ - addRequestHeader "Content-Type" "application/json" $ - setRequestBasicAuth - (encodeUtf8 cfgEmail <> "/token") - (encodeUtf8 cfgToken) $ - parseRequest_ (toString (cfgZendesk <> path)) +apiRequest :: Config -> Text -> Either String Request +apiRequest Config{..} u = mapLeft show $ catch buildRequest Left where + buildRequest :: MonadThrow m => m Request + buildRequest = do + req <- parseRequest (toString (cfgZendesk <> path)) + return $ setRequestPath (encodeUtf8 path) $ + addRequestHeader "Content-Type" "application/json" $ + setRequestBasicAuth + (encodeUtf8 cfgEmail <> "/token") + (encodeUtf8 cfgToken) $ req path :: Text path = "/api/v2" <> u @@ -83,17 +88,18 @@ addJsonBody body req = mapLeft show $ catch updateReq Left -- | Make an api call -- TODO(ks): Switch to @Either@. apiCall - :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) + :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request - -> m a + -> m (Either String a) apiCall parser req = do putTextLn $ show req v <- getResponseBody <$> httpJSON req - case parseEither parser v of - Right o -> pure o - Left e -> throwM $ MkJSONParsingException $ - "couldn't parse response " <> toText e <> "\n" <> decodeUtf8 (encode v) + pure $ parseEither parser v + -- case parseEither parser v of + -- Right o -> pure o + -- Left e -> Left $ MkJSONParsingException $ + -- "couldn't parse response " <> toText e <> "\n" <> decodeUtf8 (encode v) -- | Make a safe api call. apiCallSafe From d7cc04808b6386863d7c564b4d85ab7c083ff32c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 1 Aug 2018 16:27:05 +0200 Subject: [PATCH 11/23] WIP: Resolved a number of compile errors, but still some left --- src/DataSource/Http.hs | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 22aea96..884b8a9 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -95,7 +95,7 @@ getTicketInfo ticketId = -- | Return list of deleted tickets. listDeletedTickets - :: forall m. (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => m [DeletedTicket] listDeletedTickets = do cfg <- ask @@ -103,11 +103,11 @@ listDeletedTickets = do let url = showURL $ DeletedTicketsURL let req = apiRequest cfg url - iteratePages req + wrapIteratePages req -- | Return list of ticketIds that has been requested by config user. listRequestedTickets - :: forall m. (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => UserId -> m [TicketInfo] listRequestedTickets userId = do @@ -116,11 +116,11 @@ listRequestedTickets userId = do let url = showURL $ UserRequestedTicketsURL userId let req = apiRequest cfg url - iteratePages req + wrapIteratePages req -- | Return list of ticketIds that has been assigned by config user. listAssignedTickets - :: forall m. (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => UserId -> m [TicketInfo] listAssignedTickets userId = do @@ -129,11 +129,11 @@ listAssignedTickets userId = do let url = showURL $ UserAssignedTicketsURL userId let req = apiRequest cfg url - iteratePages req + wrapIteratePages req -- | Return list of ticketIds that has been unassigned. listUnassignedTickets - :: forall m. (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => m [TicketInfo] listUnassignedTickets = do cfg <- ask @@ -141,17 +141,17 @@ listUnassignedTickets = do let url = showURL $ UserUnassignedTicketsURL let req = apiRequest cfg url - iteratePages req + wrapIteratePages req listAdminAgents - :: forall m. (MonadIO m, MonadReader Config m) + :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => m [User] listAdminAgents = do cfg <- ask let url = showURL AgentGroupURL let req = apiRequest cfg url - iteratePages req + wrapIteratePages req -- | Export tickets from Zendesk - https://developer.zendesk.com/rest_api/docs/core/incremental_export -- NOTE: If count is less than 1000, then stop paginating. @@ -211,10 +211,13 @@ postTicketComment ticketInfo zendeskResponse = do let responseTicket = createResponseTicket (cfgAgentId cfg) ticketInfo zendeskResponse let url = showURL $ TicketsURL (zrTicketId zendeskResponse) - let req = addJsonBody responseTicket (apiRequest cfg url) + -- either (throwM . JSONEncodingException . pack) + let req = apiRequest cfg url case req of Left e -> throwM $ JSONEncodingException $ pack e - Right r -> void $ apiCall (pure . encodeToLazyText) r + Right r -> do + r' <- addJsonBody responseTicket r + void $ apiCall (pure . encodeToLazyText) r' -- | Create response ticket @@ -292,6 +295,13 @@ iteratePages -> m [a] iteratePages req = iteratePagesWithDelay 0 req +-- | Wraps a call to iteratePages with error handling for a failed request +wrapIteratePages + :: (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) + => Either String Request + -> m [a] +wrapIteratePages = either (throwM . JSONParsingException . pack) iteratePages + -- | Iterate all the ticket pages and combine into a result. Wait for -- some time in-between the requests. iteratePagesWithDelay From d9526cf3f8bc6e199a103bdf675d9035ef5c33ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Thu, 2 Aug 2018 15:05:16 +0200 Subject: [PATCH 12/23] WIP: exception handling --- src/HttpLayer.hs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index cf1cee6..80f8cde 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -56,17 +56,19 @@ emptyHTTPNetworkLayer = HTTPNetworkLayer ------------------------------------------------------------ -- | General api request function -apiRequest :: Config -> Text -> Either String Request +apiRequest :: forall m. MonadThrow m => Config -> Text -> Either String Request apiRequest Config{..} u = mapLeft show $ catch buildRequest Left where - buildRequest :: MonadThrow m => m Request + buildRequest :: m Request buildRequest = do - req <- parseRequest (toString (cfgZendesk <> path)) + req <- parseRequest (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseRequest can throw return $ setRequestPath (encodeUtf8 path) $ addRequestHeader "Content-Type" "application/json" $ setRequestBasicAuth (encodeUtf8 cfgEmail <> "/token") (encodeUtf8 cfgToken) $ req + catchException :: Exception e => e -> Either String Request + catchException (JSONParseException s) = Left s path :: Text path = "/api/v2" <> u From 6ffb37e6bdff28c97f046f60e74e5150b57aa8ee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 6 Aug 2018 13:12:50 +0200 Subject: [PATCH 13/23] WIP: replaces catch with catches --- cabal2nix.nix | 2 +- log-classifier.cabal | 1 + src/HttpLayer.hs | 45 +++++++++++++++++++++++++++----------------- 3 files changed, 30 insertions(+), 18 deletions(-) diff --git a/cabal2nix.nix b/cabal2nix.nix index 8dbd49a..299743e 100644 --- a/cabal2nix.nix +++ b/cabal2nix.nix @@ -1,7 +1,7 @@ { mkDerivation, aeson, array, attoparsec, base, bytestring , containers, directory, either, generics-sop, hspec, http-conduit , monad-control, mtl, optparse-applicative, QuickCheck, reflection -, regex-tdfa, resource-pool, sqlite-simple, stdenv, text, time +, regex-tdfa, resource-pool, safe-exceptions, sqlite-simple, stdenv, text, time , transformers-base, universum, unliftio, zip-archive }: mkDerivation { diff --git a/log-classifier.cabal b/log-classifier.cabal index 34bb297..23a62d5 100644 --- a/log-classifier.cabal +++ b/log-classifier.cabal @@ -57,6 +57,7 @@ library , universum , zip-archive , QuickCheck + , safe-exceptions -- Database backend , sqlite-simple , resource-pool diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index 80f8cde..99486f0 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -9,29 +9,32 @@ module HttpLayer import Universum -import Data.Aeson (FromJSON, ToJSON, Value, encode) +import Control.Exception.Safe + ( catches + , Handler (..) + ) +import Data.Aeson (FromJSON, ToJSON, Value) import Data.Aeson.Types (Parser, parseEither) import Data.Either.Combinators (mapLeft) -import Data.Text (unpack) import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, parseRequest_, setRequestBasicAuth, setRequestBodyJSON, - setRequestMethod, setRequestPath, parseRequest) + setRequestMethod, setRequestPath, parseRequest, + JSONException(..)) import DataSource.Types (Config (..), HTTPNetworkLayer (..)) -import qualified Prelude (Show(..)) ------------------------------------------------------------ -- JSON parsing exceptions ------------------------------------------------------------ -- | Exceptions that occur during JSON parsing -data JSONException - = JSONParsingException Text - | JSONEncodingException Text - -instance Exception JSONException -instance Prelude.Show JSONException where - show (JSONParsingException s) = "JSON parsing exception: " <> (unpack s) - show (JSONEncodingException s) = "JSON encoding exception: " <> (unpack s) +-- data JSONException +-- = JSONParsingException Text +-- | JSONEncodingException Text +-- +-- instance Exception JSONException +-- instance Prelude.Show JSONException where +-- show (JSONParsingException s) = "JSON parsing exception: " <> (unpack s) +-- show (JSONEncodingException s) = "JSON encoding exception: " <> (unpack s) ------------------------------------------------------------ -- Layer @@ -56,10 +59,13 @@ emptyHTTPNetworkLayer = HTTPNetworkLayer ------------------------------------------------------------ -- | General api request function -apiRequest :: forall m. MonadThrow m => Config -> Text -> Either String Request -apiRequest Config{..} u = mapLeft show $ catch buildRequest Left +apiRequest + :: Config + -> Text + -> Either String Request +apiRequest Config{..} u = mapLeft show $ catches buildRequest handlerList --catchException where - buildRequest :: m Request + buildRequest :: forall m. MonadThrow m => m Request buildRequest = do req <- parseRequest (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseRequest can throw return $ setRequestPath (encodeUtf8 path) $ @@ -67,8 +73,13 @@ apiRequest Config{..} u = mapLeft show $ catch buildRequest Left setRequestBasicAuth (encodeUtf8 cfgEmail <> "/token") (encodeUtf8 cfgToken) $ req - catchException :: Exception e => e -> Either String Request - catchException (JSONParseException s) = Left s + handlerList :: MonadThrow m => [Handler m Request] + handlerList = [handlerJSON] + handlerJSON :: MonadThrow m => Handler m Request + handlerJSON = Handler $ \(ex :: JSONException) -> throwM ex + -- here we're catching only synchronous exceptions + -- catchException :: (Exception e, MonadThrow m) => e -> m Request + -- catchException p@(JSONParseException _ _ _) = throwM p path :: Text path = "/api/v2" <> u From 24cbeddb7b55df20171f1455049a852183104089 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 6 Aug 2018 13:51:31 +0200 Subject: [PATCH 14/23] WIP: adds parseUrlThrow instead of parseRequest --- src/HttpLayer.hs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index 99486f0..6916f7c 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -10,15 +10,22 @@ module HttpLayer import Universum import Control.Exception.Safe - ( catches + ( + catches , Handler (..) + , throwM ) import Data.Aeson (FromJSON, ToJSON, Value) import Data.Aeson.Types (Parser, parseEither) import Data.Either.Combinators (mapLeft) +import Network.HTTP.Client.Conduit + ( + HttpException (..) + , parseUrlThrow + ) import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, parseRequest_, setRequestBasicAuth, setRequestBodyJSON, - setRequestMethod, setRequestPath, parseRequest, + setRequestMethod, setRequestPath, JSONException(..)) import DataSource.Types (Config (..), HTTPNetworkLayer (..)) @@ -67,16 +74,21 @@ apiRequest Config{..} u = mapLeft show $ catches buildRequest handlerList --catc where buildRequest :: forall m. MonadThrow m => m Request buildRequest = do - req <- parseRequest (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseRequest can throw + req <- parseUrlThrow (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseRequest can throw return $ setRequestPath (encodeUtf8 path) $ addRequestHeader "Content-Type" "application/json" $ setRequestBasicAuth (encodeUtf8 cfgEmail <> "/token") (encodeUtf8 cfgToken) $ req handlerList :: MonadThrow m => [Handler m Request] - handlerList = [handlerJSON] + handlerList = [ + handlerJSON + , handlerHTTP + ] handlerJSON :: MonadThrow m => Handler m Request handlerJSON = Handler $ \(ex :: JSONException) -> throwM ex + handlerHTTP :: MonadThrow m => Handler m Request + handlerHTTP = Handler $ \(ex :: HttpException) -> throwM ex -- here we're catching only synchronous exceptions -- catchException :: (Exception e, MonadThrow m) => e -> m Request -- catchException p@(JSONParseException _ _ _) = throwM p From 16c9584d8e3d2fcbc18b7407e2721ca9a44d03df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 6 Aug 2018 16:06:14 +0200 Subject: [PATCH 15/23] WIP: Some more progress with removing compilation errors --- src/DataSource/Http.hs | 58 ++++++++++++++++++++++++++++-------------- src/HttpLayer.hs | 9 ++++--- 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 477da9b..14babc1 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -11,12 +11,14 @@ module DataSource.Http import Universum import Control.Concurrent (threadDelay) +import Control.Exception.Safe (throwM) import Control.Monad.Reader (ask) import Data.Aeson (parseJSON) import Data.Aeson.Text (encodeToLazyText) import Data.List (nub) import Data.Text (pack) +import Network.HTTP.Client.Conduit (HttpException (..)) import Network.HTTP.Simple (Request, getResponseBody, httpLBS, parseRequest_) import HttpLayer (HTTPNetworkLayer (..), apiRequest, apiRequestAbsolute, @@ -88,10 +90,11 @@ getTicketInfo ticketId = let url = showURL $ TicketsURL ticketId let req = apiRequest cfg url - - apiCall <- asksHTTPNetworkLayer hnlApiCall - - rightToMaybe <$> apiCall parseTicket req + case req of + Left e -> return Nothing -- TODO(md): see how to propagate 'e' + Right r -> do + apiCall <- asksHTTPNetworkLayer hnlApiCall + rightToMaybe <$> apiCall parseTicket r -- | Return list of deleted tickets. listDeletedTickets @@ -168,12 +171,20 @@ getExportedTickets time = do let url = showURL $ ExportDataByTimestamp time let req = apiRequestAbsolute cfg url - iterateExportedTicketsWithDelay req (apiCall parseJSON) + -- iterateExportedTicketsWithDelay req (apiCall parseJSON) + wrappedIterate req (apiCall parseJSON) where + wrappedIterate + :: Either String Request + -> (Request -> m (Either String (PageResultList TicketInfo))) + -> m [TicketInfo] + wrappedIterate (Left e) _ = throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + wrappedIterate (Right r) f = iterateExportedTicketsWithDelay r f + iterateExportedTicketsWithDelay :: Request - -> (Request -> m (PageResultList TicketInfo)) + -> (Request -> m (Either String (PageResultList TicketInfo))) -> m [TicketInfo] iterateExportedTicketsWithDelay req apiCall = do cfg <- ask @@ -183,19 +194,28 @@ getExportedTickets time = do liftIO $ threadDelay $ 10 * 1000000 -- Wait, Zendesk allows for 10 per minute. let req' = apiRequestAbsolute cfg nextPage' - (PageResultList pagen nextPagen count) <- apiCall req' - case nextPagen of - Just nextUrl -> if maybe False (>= 1000) count - then go (list' <> pagen) nextUrl - else pure (list' <> pagen) - - Nothing -> pure (list' <> pagen) - - - (PageResultList page0 nextPage _) <- apiCall req - case nextPage of - Just nextUrl -> go page0 nextUrl - Nothing -> pure page0 + case req' of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right req'' -> do + req''' <- apiCall req'' + case req''' of + Right (PageResultList pagen nextPagen count) -> + case nextPagen of + Just nextUrl -> if maybe False (>= 1000) count + then go (list' <> pagen) nextUrl + else pure (list' <> pagen) + + Nothing -> pure (list' <> pagen) + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + + + req' <- apiCall req + case req' of + Right (PageResultList page0 nextPage _) -> do + case nextPage of + Just nextUrl -> go page0 nextUrl + Nothing -> pure page0 + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception -- | Send API request to post comment postTicketComment diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index 6916f7c..f6d2107 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -74,7 +74,7 @@ apiRequest Config{..} u = mapLeft show $ catches buildRequest handlerList --catc where buildRequest :: forall m. MonadThrow m => m Request buildRequest = do - req <- parseUrlThrow (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseRequest can throw + req <- parseUrlThrow (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseUrlThrow can throw return $ setRequestPath (encodeUtf8 path) $ addRequestHeader "Content-Type" "application/json" $ setRequestBasicAuth @@ -96,9 +96,12 @@ apiRequest Config{..} u = mapLeft show $ catches buildRequest handlerList --catc path = "/api/v2" <> u -- | Api request but use absolute path -apiRequestAbsolute :: Config -> Text -> Request +apiRequestAbsolute + :: Config + -> Text + -> Either String Request apiRequestAbsolute Config{..} u = - addRequestHeader "Content-Type" "application/json" $ + Right <$> addRequestHeader "Content-Type" "application/json" $ setRequestBasicAuth (encodeUtf8 cfgEmail <> "/token") (encodeUtf8 cfgToken) $ parseRequest_(toString u) From 8569110deed7cdf94639abc79202515b87a13eba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 6 Aug 2018 16:19:43 +0200 Subject: [PATCH 16/23] WIP: Yet some more progress with removing compilation errors --- src/DataSource/Http.hs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 14babc1..547c1b7 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -234,10 +234,12 @@ postTicketComment ticketInfo zendeskResponse = do -- either (throwM . JSONEncodingException . pack) let req = apiRequest cfg url case req of - Left e -> throwM $ JSONEncodingException $ pack e - Right r -> do - r' <- addJsonBody responseTicket r - void $ apiCall (pure . encodeToLazyText) r' + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right r -> do + let r' = addJsonBody responseTicket r + case r' of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right r'' -> void $ apiCall (pure . encodeToLazyText) r'' -- | Create response ticket From 35c278be42a16288c872583878e8f70232066021 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Mon, 6 Aug 2018 16:24:51 +0200 Subject: [PATCH 17/23] WIP: Yet some more progress with removing compilation errors --- src/DataSource/Http.hs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 547c1b7..0f81500 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -275,7 +275,7 @@ _getUser = do let url = showURL UserInfoURL let req = apiRequest cfg url case req of - Left e -> throwM $ JSONParsingException e + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception Right r -> apiCall parseJSON r -- | Given attachmentUrl, return attachment in bytestring From afcbd80bfe692e41169ebb97a6523de497c8500c Mon Sep 17 00:00:00 2001 From: Rob Cohen Date: Mon, 6 Aug 2018 16:42:43 -0400 Subject: [PATCH 18/23] [TSD-85] - Additional imports require new cabal2nix.nix --- cabal2nix.nix | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cabal2nix.nix b/cabal2nix.nix index 299743e..063001b 100644 --- a/cabal2nix.nix +++ b/cabal2nix.nix @@ -1,8 +1,8 @@ { mkDerivation, aeson, array, attoparsec, base, bytestring , containers, directory, either, generics-sop, hspec, http-conduit , monad-control, mtl, optparse-applicative, QuickCheck, reflection -, regex-tdfa, resource-pool, safe-exceptions, sqlite-simple, stdenv, text, time -, transformers-base, universum, unliftio, zip-archive +, regex-tdfa, resource-pool, safe-exceptions, sqlite-simple, stdenv +, text, time, transformers-base, universum, unliftio, zip-archive }: mkDerivation { pname = "log-classifier"; @@ -11,10 +11,11 @@ mkDerivation { isLibrary = true; isExecutable = true; libraryHaskellDepends = [ - aeson array attoparsec base bytestring containers directory + aeson array attoparsec base bytestring containers directory either generics-sop http-conduit monad-control mtl optparse-applicative - QuickCheck reflection regex-tdfa resource-pool sqlite-simple text - time transformers-base universum unliftio zip-archive + QuickCheck reflection regex-tdfa resource-pool safe-exceptions + sqlite-simple text time transformers-base universum unliftio + zip-archive ]; executableHaskellDepends = [ base universum ]; testHaskellDepends = [ From c73bb573dd4b60084cb6595fd70d3611a4990d43 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 7 Aug 2018 14:58:11 +0200 Subject: [PATCH 19/23] WIP: almost compiles --- src/DataSource/Http.hs | 43 ++++++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 0f81500..2ee9d2c 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -276,7 +276,11 @@ _getUser = do let req = apiRequest cfg url case req of Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception - Right r -> apiCall parseJSON r + Right r -> do + r' <- apiCall parseJSON r + case r' of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right u -> pure u -- | Given attachmentUrl, return attachment in bytestring getAttachment @@ -290,7 +294,7 @@ getAttachment Attachment{..} = Just . AttachmentContent . getResponseBody <$> ht -- | Get ticket's comments getTicketComments - :: (MonadIO m, MonadReader Config m) + :: (MonadIO m, MonadReader Config m, MonadThrow m) => TicketId -> m [Comment] getTicketComments tId = do @@ -300,14 +304,16 @@ getTicketComments tId = do let url = showURL $ TicketCommentsURL tId let req = apiRequest cfg url + case req of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right r -> do + result <- apiCallSafe parseComments r - result <- apiCallSafe parseComments req - - -- TODO(ks): For now return empty if there is an exception. - -- After we have exception handling, we propagate this up. - case result of - Left _ -> pure [] - Right r -> pure r + -- TODO(ks): For now return empty if there is an exception. + -- After we have exception handling, we propagate this up. + case result of + Left _ -> pure [] + Right r -> pure r ------------------------------------------------------------ -- Utility @@ -325,7 +331,10 @@ wrapIteratePages :: (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) => Either String Request -> m [a] -wrapIteratePages = either (throwM . JSONParsingException . pack) iteratePages +wrapIteratePages = either (throwM . throwFun) iteratePages + where + throwFun :: String -> HttpException -- TODO(md): Fix this HttpException type to an appropriate one + throwFun _ = InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception -- | Iterate all the ticket pages and combine into a result. Wait for -- some time in-between the requests. @@ -345,10 +354,16 @@ iteratePagesWithDelay seconds req = do threadDelay $ seconds * 1000000 let req' = apiRequestAbsolute cfg nextPage' - (PageResultList pagen nextPagen _) <- apiCall parseJSON req' - case nextPagen of - Just nextUrl -> go (list' <> pagen) nextUrl - Nothing -> pure (list' <> pagen) + case req' of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right req'' -> do + req''' <- apiCall parseJSON req'' + case req''' of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right (PageResultList pagen nextPagen _) -> do + case nextPagen of + Just nextUrl -> go (list' <> pagen) nextUrl + Nothing -> pure (list' <> pagen) (PageResultList page0 nextPage _) <- liftIO $ apiCall parseJSON req case nextPage of From 4862c0d3cfb0f6d7e4254b704c9468fd15905a48 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Tue, 7 Aug 2018 15:09:26 +0200 Subject: [PATCH 20/23] WIP: It finally compiles --- src/DataSource/Http.hs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 2ee9d2c..b1d6a1a 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -321,7 +321,7 @@ getTicketComments tId = do -- | Iterate all the ticket pages and combine into a result. iteratePages - :: forall m a. (MonadIO m, MonadReader Config m, FromPageResultList a) + :: forall m a. (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) => Request -> m [a] iteratePages req = iteratePagesWithDelay 0 req @@ -339,7 +339,7 @@ wrapIteratePages = either (throwM . throwFun) iteratePages -- | Iterate all the ticket pages and combine into a result. Wait for -- some time in-between the requests. iteratePagesWithDelay - :: forall m a. (MonadIO m, MonadReader Config m, FromPageResultList a) + :: forall m a. (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) => Int -> Request -> m [a] @@ -365,7 +365,11 @@ iteratePagesWithDelay seconds req = do Just nextUrl -> go (list' <> pagen) nextUrl Nothing -> pure (list' <> pagen) - (PageResultList page0 nextPage _) <- liftIO $ apiCall parseJSON req - case nextPage of - Just nextUrl -> liftIO $ go page0 nextUrl - Nothing -> pure page0 + -- <- liftIO $ apiCall parseJSON req + req' <- liftIO $ apiCall parseJSON req + case req' of + Left e -> throwM $ InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + Right (PageResultList page0 nextPage _) -> do + case nextPage of + Just nextUrl -> liftIO $ go page0 nextUrl + Nothing -> pure page0 From 088f0b78aafbd07f53e35983bc79522bd75c586e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20Dimja=C5=A1evi=C4=87?= Date: Wed, 8 Aug 2018 16:28:02 +0200 Subject: [PATCH 21/23] WIP: Changing Either String a to (MonadThrow m => m a) --- src/DataSource/Http.hs | 36 ++++++++----- src/DataSource/Types.hs | 6 +-- src/HttpLayer.hs | 113 +++++++++++++++++----------------------- 3 files changed, 74 insertions(+), 81 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index b1d6a1a..689f6d0 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -11,7 +11,7 @@ module DataSource.Http import Universum import Control.Concurrent (threadDelay) -import Control.Exception.Safe (throwM) +import Control.Exception.Safe (catches, throwM, Handler (..)) import Control.Monad.Reader (ask) import Data.Aeson (parseJSON) @@ -78,35 +78,43 @@ emptyDataLayer = DataLayer -- | Get single ticket info. getTicketInfo - :: forall m. (HasCallStack, MonadIO m, MonadReader Config m, MonadCatch m) + :: forall m. (MonadIO m, MonadReader Config m, MonadCatch m) => TicketId -> m (Maybe TicketInfo) getTicketInfo ticketId = - catch getInfo $ \(e :: JSONException) -> throwM e + catches getInfo handlerList where getInfo :: m (Maybe TicketInfo) getInfo = do cfg <- ask let url = showURL $ TicketsURL ticketId - let req = apiRequest cfg url - case req of - Left e -> return Nothing -- TODO(md): see how to propagate 'e' - Right r -> do - apiCall <- asksHTTPNetworkLayer hnlApiCall - rightToMaybe <$> apiCall parseTicket r + req <- apiRequest cfg url + + apiCall <- asksHTTPNetworkLayer hnlApiCall + Just <$> apiCall parseTicket req + + handlerList :: [Handler m (Maybe TicketInfo)] + handlerList = [handlerJSON, handlerHTTP] + handlerJSON :: Handler m (Maybe TicketInfo) + handlerJSON = Handler $ \(ex :: JSONException) -> return Nothing + handlerHTTP :: Handler m (Maybe TicketInfo) + handlerHTTP = Handler $ \(ex :: HttpException) -> return Nothing -- | Return list of deleted tickets. listDeletedTickets :: forall m. (MonadIO m, MonadReader Config m, MonadThrow m) => m [DeletedTicket] -listDeletedTickets = do - cfg <- ask +listDeletedTickets = go + where + go :: m [DeletedTicket] + go = do + cfg <- ask - let url = showURL $ DeletedTicketsURL - let req = apiRequest cfg url + let url = showURL $ DeletedTicketsURL + req <- apiRequest cfg url - wrapIteratePages req + wrapIteratePages req -- | Return list of ticketIds that has been requested by config user. listRequestedTickets diff --git a/src/DataSource/Types.hs b/src/DataSource/Types.hs index 53fc33c..af03655 100644 --- a/src/DataSource/Types.hs +++ b/src/DataSource/Types.hs @@ -204,10 +204,10 @@ data DataLayer m = DataLayer -- the @Http@ module, say in the @ZendeskaLayer@, but it's good enough for now. -- We need to use RankNTypes due to some complications that appeared. data HTTPNetworkLayer = HTTPNetworkLayer - { hnlAddJsonBody :: forall a. (ToJSON a) => a -> Request -> Either String Request + { hnlAddJsonBody :: forall m a. (ToJSON a, MonadThrow m) => a -> Request -> m Request -- TODO(ks): These two below are basically the same, will fix in future PR, requires refactoring. - , hnlApiCall :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) - , hnlApiCallSafe :: forall m a. (MonadIO m, FromJSON a) => (Value -> Parser a) -> Request -> m (Either String a) + , hnlApiCall :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request -> m a + , hnlApiCallSafe :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request -> m a } -- | The IOLayer interface that we can expose. diff --git a/src/HttpLayer.hs b/src/HttpLayer.hs index f6d2107..d36f070 100644 --- a/src/HttpLayer.hs +++ b/src/HttpLayer.hs @@ -9,39 +9,25 @@ module HttpLayer import Universum -import Control.Exception.Safe - ( - catches - , Handler (..) - , throwM - ) +import Control.Exception.Safe (throwM) import Data.Aeson (FromJSON, ToJSON, Value) import Data.Aeson.Types (Parser, parseEither) -import Data.Either.Combinators (mapLeft) -import Network.HTTP.Client.Conduit - ( - HttpException (..) - , parseUrlThrow - ) -import Network.HTTP.Simple (Request, addRequestHeader, getResponseBody, httpJSON, - parseRequest_, setRequestBasicAuth, setRequestBodyJSON, - setRequestMethod, setRequestPath, - JSONException(..)) +import Data.Text (pack) +import Network.HTTP.Client.Conduit (parseUrlThrow) +import Network.HTTP.Simple (JSONException (..), Request, addRequestHeader, + getResponseBody, httpJSON, parseRequest_, setRequestBasicAuth, + setRequestBodyJSON, setRequestMethod, setRequestPath) import DataSource.Types (Config (..), HTTPNetworkLayer (..)) + ------------------------------------------------------------ --- JSON parsing exceptions +-- Internal Exceptions ------------------------------------------------------------ --- | Exceptions that occur during JSON parsing --- data JSONException --- = JSONParsingException Text --- | JSONEncodingException Text --- --- instance Exception JSONException --- instance Prelude.Show JSONException where --- show (JSONParsingException s) = "JSON parsing exception: " <> (unpack s) --- show (JSONEncodingException s) = "JSON encoding exception: " <> (unpack s) +newtype APICallException + = MkAPICallException Text deriving Show + +instance Exception APICallException ------------------------------------------------------------ -- Layer @@ -66,32 +52,29 @@ emptyHTTPNetworkLayer = HTTPNetworkLayer ------------------------------------------------------------ -- | General api request function +-- This function can throw a 'JSONException' and an 'HttpException'. apiRequest - :: Config + :: MonadThrow m + => Config -> Text - -> Either String Request -apiRequest Config{..} u = mapLeft show $ catches buildRequest handlerList --catchException + -> m Request +apiRequest Config{..} u = do + req <- parseUrlThrow (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseUrlThrow can throw + return $ setRequestPath (encodeUtf8 path) $ + addRequestHeader "Content-Type" "application/json" $ + setRequestBasicAuth + (encodeUtf8 cfgEmail <> "/token") + (encodeUtf8 cfgToken) $ req where - buildRequest :: forall m. MonadThrow m => m Request - buildRequest = do - req <- parseUrlThrow (toString (cfgZendesk <> path)) -- TODO(md): Get a list of exceptions that parseUrlThrow can throw - return $ setRequestPath (encodeUtf8 path) $ - addRequestHeader "Content-Type" "application/json" $ - setRequestBasicAuth - (encodeUtf8 cfgEmail <> "/token") - (encodeUtf8 cfgToken) $ req - handlerList :: MonadThrow m => [Handler m Request] - handlerList = [ - handlerJSON - , handlerHTTP - ] - handlerJSON :: MonadThrow m => Handler m Request - handlerJSON = Handler $ \(ex :: JSONException) -> throwM ex - handlerHTTP :: MonadThrow m => Handler m Request - handlerHTTP = Handler $ \(ex :: HttpException) -> throwM ex - -- here we're catching only synchronous exceptions - -- catchException :: (Exception e, MonadThrow m) => e -> m Request - -- catchException p@(JSONParseException _ _ _) = throwM p + -- handlerList :: [Handler m Request] + -- handlerList = [ + -- handlerJSON + -- , handlerHTTP + -- ] + -- handlerJSON :: Handler m Request + -- handlerJSON = Handler $ \(ex :: JSONException) -> throwM ex + -- handlerHTTP :: Handler m Request + -- handlerHTTP = Handler $ \(ex :: HttpException) -> throwM ex path :: Text path = "/api/v2" <> u @@ -106,24 +89,29 @@ apiRequestAbsolute Config{..} u = parseRequest_(toString u) -- | Request PUT -addJsonBody :: forall a. ToJSON a => a -> Request -> Either String Request -addJsonBody body req = mapLeft show $ catch updateReq Left - where - updateReq :: Exception e => Either e Request - updateReq = Right <$> setRequestBodyJSON body $ setRequestMethod "PUT" req +addJsonBody + :: forall m a. (ToJSON a, MonadThrow m) + => a + -> Request + -> m Request +addJsonBody body req = + return <$> setRequestBodyJSON body $ setRequestMethod "PUT" req -- | Make an api call --- TODO(ks): Switch to @Either@. apiCall - :: forall m a. (MonadIO m, FromJSON a) + :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request - -> m (Either String a) + -> m a apiCall parser req = do putTextLn $ show req v <- getResponseBody <$> httpJSON req - pure $ parseEither parser v + either (throwEx v) return (parseEither parser v) + where + throwEx :: Value -> String -> m a + throwEx v msg = throwM $ MkAPICallException . pack $ + "Exception occured when parsing " ++ (show v) ++ ": " ++ msg -- case parseEither parser v of -- Right o -> pure o -- Left e -> Left $ MkJSONParsingException $ @@ -131,11 +119,8 @@ apiCall parser req = do -- | Make a safe api call. apiCallSafe - :: forall m a. (MonadIO m, FromJSON a) + :: forall m a. (MonadIO m, MonadThrow m, FromJSON a) => (Value -> Parser a) -> Request - -> m (Either String a) -apiCallSafe parser req = do - putTextLn $ show req - v <- getResponseBody <$> httpJSON req - pure $ parseEither parser v + -> m a +apiCallSafe = apiCall From 753c1fb45dad8c9c4e427cb6235855e96fe75c0e Mon Sep 17 00:00:00 2001 From: Rob Cohen Date: Thu, 9 Aug 2018 00:07:37 -0400 Subject: [PATCH 22/23] [TSD-85] - Began rewriting iteratePages to use MonadThrow etc --- src/DataSource/Http.hs | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 689f6d0..04fbb7c 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -11,7 +11,7 @@ module DataSource.Http import Universum import Control.Concurrent (threadDelay) -import Control.Exception.Safe (catches, throwM, Handler (..)) +import Control.Exception.Safe (Handler (..), catches, throwM) import Control.Monad.Reader (ask) import Data.Aeson (parseJSON) @@ -21,8 +21,8 @@ import Data.Text (pack) import Network.HTTP.Client.Conduit (HttpException (..)) import Network.HTTP.Simple (Request, getResponseBody, httpLBS, parseRequest_) -import HttpLayer (HTTPNetworkLayer (..), apiRequest, apiRequestAbsolute, - JSONException (..)) +import HttpLayer (HTTPNetworkLayer (..), JSONException (..), apiRequest, + apiRequestAbsolute) import DataSource.Types (Attachment (..), AttachmentContent (..), Comment (..), CommentBody (..), CommentId (..), Config (..), DataLayer (..), @@ -331,18 +331,11 @@ getTicketComments tId = do iteratePages :: forall m a. (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) => Request - -> m [a] -iteratePages req = iteratePagesWithDelay 0 req - --- | Wraps a call to iteratePages with error handling for a failed request -wrapIteratePages - :: (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) - => Either String Request - -> m [a] -wrapIteratePages = either (throwM . throwFun) iteratePages + -> m [Maybe a] +iteratePages req = catches (iteratePagesWithDelay 0 req) handlerList where - throwFun :: String -> HttpException -- TODO(md): Fix this HttpException type to an appropriate one - throwFun _ = InvalidUrlException "" "" -- TODO(md): See how to convert a String 'e' to an appropriate exception + handlerList :: Handler m (Maybe a) + handlerList = Handler $ \(ex :: InvalidUrlException) -> return Nothing -- | Iterate all the ticket pages and combine into a result. Wait for -- some time in-between the requests. From 7f8fadcf3ca481fac9f00c734e152e02421a00ad Mon Sep 17 00:00:00 2001 From: Rob Cohen Date: Thu, 9 Aug 2018 00:24:08 -0400 Subject: [PATCH 23/23] [TSD-85] - Minor edits, still having some type errors --- src/DataSource/Http.hs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/DataSource/Http.hs b/src/DataSource/Http.hs index 04fbb7c..19c6a21 100644 --- a/src/DataSource/Http.hs +++ b/src/DataSource/Http.hs @@ -331,11 +331,13 @@ getTicketComments tId = do iteratePages :: forall m a. (MonadIO m, MonadReader Config m, MonadThrow m, FromPageResultList a) => Request - -> m [Maybe a] -iteratePages req = catches (iteratePagesWithDelay 0 req) handlerList + -> m (Maybe [Request]) +iteratePages req = catches tryIteratePages handlerList where - handlerList :: Handler m (Maybe a) - handlerList = Handler $ \(ex :: InvalidUrlException) -> return Nothing + tryIteratePages = iteratePagesWithDelay 0 req + handlerList :: Handler m (Maybe Request) + handlerList = handlerInvalidUrl + handlerInvalidUrl = Handler $ \(ex :: InvalidUrlException ) -> return Nothing -- | Iterate all the ticket pages and combine into a result. Wait for -- some time in-between the requests.