Skip to content

Commit 7186913

Browse files
iohk-bors[bot]KtorZIOHK
authored
Merge #1623
1623: Mock metadata aggregation server r=KtorZ a=KtorZ # Issue Number <!-- Put here a reference to the issue this PR relates to and which requirements it tackles --> #1597 # Overview <!-- Detail in a few bullet points the work accomplished in this PR --> - 64052c5 📍 **Basic skeleton for refreshing metadata.** Still requires proper log messages and error handling. But this already allows some level of testing. - 7be9a83 📍 **log events while refreshing metadata** - 566d95d 📍 **handle 404 in the metadata client directly** So that the calling code needs not to worry about the 'ClientError'. Any error becomes unexpected and all errors can be treated consistently. - 398bb78 📍 **merge 'refresh' and 'Client' so that the client is already caching results by itself.** This makes it slightly easier to use from a consumer perspective. Once one has a handle on a 'Client IO Api' it can makes requests without having to worry about whether they get cached or not. Doing so, I also removed the 'ClientError' from the response. The client does handle error by itself and log messages accordingly. A caller would likely not do anything more than relogging the error. Plus, from the caller perspective, a metadata not being there and the request failing has pretty much the same end: no metadata. - 98a499d 📍 **write small unit / property tests to exercise the client.** Written as property so we get some fuzzy testing and test cases with and without metadata Fails nicely with each log messages obtained in the run: ``` Assertion failed (after 1 test): PoolId {getPoolId = "\SOH\NUL\SOH\NUL\NUL\NUL\NUL\NUL\NUL\SOH\SOH\NUL\SOH\SOH\SOH\SOH\NUL\SOH\NUL\SOH\SOH\SOH\NUL\SOH\NUL\SOH\NUL\NUL\NUL\SOH\SOH\NUL"} MsgRefreshingMetadata (PoolId {getPoolId = "\SOH\NUL\SOH\NUL\NUL\NUL\NUL\NUL\NUL\SOH\SOH\NUL\SOH\SOH\SOH\SOH\NUL\SOH\NUL\SOH\SOH\SOH\NUL\SOH\NUL\SOH\NUL\NUL\NUL\SOH\SOH\NUL"}) (Just (StakePoolOffChainMetadata {ticker = StakePoolTicker {unStakePoolTicker = "VUHQ"}, name = "_-NfdG\21785\&0?-wjm'\\.G{0MtJ4E", description = "_-NfdG\21785\&0?-wjm'\\.G{0MtJ4E", homepage = "https://\tXuP9h>v\ETXXM\FSUHd\ACKp\SOH\FS\SOHg\DEL$.io"}),2020-05-04 23:31:26.917183969 UTC) MsgUsingCached (PoolId {getPoolId = "\SOH\NUL\SOH\NUL\NUL\NUL\NUL\NUL\NUL\SOH\SOH\NUL\SOH\SOH\SOH\SOH\NUL\SOH\NUL\SOH\SOH\SOH\NUL\SOH\NUL\SOH\NUL\NUL\NUL\SOH\SOH\NUL"}) 2020-05-04 23:31:26.917183969 UTC ``` - 51cf00a 📍 **add an extra scenario illustrating a unhappy path** ``` Cardano.Pool.Metadata Metadata - MockServer Mock Server works as intended +++ OK, passed 100 tests: 51% Got Valid Metadata 49% No Corresponding Metadata Cache metadata when called twice within the TTL +++ OK, passed 10 tests. Fetch them again when fetching outside of the TTL +++ OK, passed 10 tests. Returns 'Nothing' and a warning log message on failure +++ OK, passed 1 test. ``` - c76f29d 📍 **move 'count' to 'Test.Util.Trace' with extra comments'** <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Assign the PR to a corresponding milestone ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: KtorZ <matthias.benkort@gmail.com> Co-authored-by: IOHK <devops+stack-project@iohk.io>
2 parents 283b84d + d85ba64 commit 7186913

File tree

7 files changed

+284
-43
lines changed

7 files changed

+284
-43
lines changed

lib/core/cardano-wallet-core.cabal

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,6 @@ test-suite unit
216216
, safe
217217
, scrypt
218218
, servant
219-
, servant-client
220219
, servant-server
221220
, servant-swagger
222221
, stm

lib/core/src/Cardano/Pool/Metadata.hs

Lines changed: 142 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
{-# LANGUAGE DataKinds #-}
2+
{-# LANGUAGE DeriveGeneric #-}
3+
{-# LANGUAGE KindSignatures #-}
4+
{-# LANGUAGE LambdaCase #-}
5+
{-# LANGUAGE NamedFieldPuns #-}
26
{-# LANGUAGE TypeApplications #-}
37
{-# LANGUAGE TypeOperators #-}
48

@@ -12,10 +16,17 @@
1216
--
1317
-- - https://github.com/input-output-hk/smash
1418
--
19+
-- This module is expected to be mostly used qualified as 'Metadata' to give
20+
-- context to the exposed functions and data-types.
1521
module Cardano.Pool.Metadata
1622
( Api
23+
24+
-- * Client
1725
, Client(..)
18-
, mkClient
26+
, ClientConfig (..)
27+
, ClientCallbacks (..)
28+
, newClient
29+
, MetadataRegistryLog (..)
1930

2031
-- * Re-export
2132
, BaseUrl (..)
@@ -27,14 +38,30 @@ module Cardano.Pool.Metadata
2738

2839
import Prelude
2940

41+
import Cardano.BM.Data.Severity
42+
( Severity (..) )
43+
import Cardano.BM.Data.Tracer
44+
( HasPrivacyAnnotation (..), HasSeverityAnnotation (..) )
3045
import Cardano.Wallet.Api.Types
3146
( ApiT (..) )
3247
import Cardano.Wallet.Primitive.Types
3348
( PoolId, StakePoolOffChainMetadata (..) )
49+
import Control.Tracer
50+
( Tracer, traceWith )
3451
import Data.Proxy
3552
( Proxy (..) )
53+
import Data.Text.Class
54+
( ToText (..) )
55+
import Data.Time.Clock
56+
( NominalDiffTime, UTCTime, diffUTCTime, getCurrentTime )
57+
import Fmt
58+
( pretty )
59+
import GHC.Generics
60+
( Generic )
3661
import Network.HTTP.Client
3762
( Manager, defaultManagerSettings, newManager )
63+
import Network.HTTP.Types
64+
( status404 )
3865
import Servant
3966
( (:>), Capture, Get, JSON )
4067
import Servant.Client
@@ -44,9 +71,12 @@ import Servant.Client
4471
, Scheme (..)
4572
, client
4673
, mkClientEnv
74+
, responseStatusCode
4775
, runClientM
4876
)
4977

78+
import qualified Data.Text as T
79+
5080
--
5181
-- Api
5282
--
@@ -65,23 +95,123 @@ type Api
6595
-- | A client for fetching metadata from an Aggregation server.
6696
--
6797
-- See also 'newClient' to construct a client.
68-
newtype Client api = Client
98+
newtype Client api m = Client
6999
{ getStakePoolMetadata
70100
:: PoolId
71-
-> IO (Either ClientError StakePoolOffChainMetadata)
101+
-> m (Maybe StakePoolOffChainMetadata)
102+
}
103+
104+
-- | A configuration for managing metadata with the aggregation server.
105+
-- Callbacks and parameterized effects allows for easier testing while a real
106+
-- specialization would wire a database connector in here.
107+
data ClientConfig = ClientConfig
108+
{ manager
109+
:: Manager
110+
-- ^ An HTTP connection manager.
111+
112+
, baseUrl
113+
:: BaseUrl
114+
-- ^ Url for reaching out to the metadata aggregation server.
115+
116+
, cacheTTL
117+
:: NominalDiffTime
118+
-- ^ A constant for the maximum age of cached registry metadatabefore
119+
-- it's considered to be stale.
72120
}
73121

74-
mkClient
75-
:: Manager
76-
-> BaseUrl
77-
-> Client Api
78-
mkClient mgr baseUrl = Client
79-
{ getStakePoolMetadata = \pid ->
80-
fmap getApiT <$> run (getMetadata (ApiT pid))
122+
-- | Callbacks interfaces allowing the client to cache and manage cached
123+
-- entities. These would typically be hooked up with a database.
124+
data ClientCallbacks (m :: * -> *) = ClientCallbacks
125+
{ saveMetadata
126+
:: PoolId -> (Maybe StakePoolOffChainMetadata, UTCTime) -> m ()
127+
-- ^ A callback action for storing an off-chain metadata. The callback
128+
-- may be called with 'Nothing' to store that no metadata were found for
129+
-- a particular 'PoolId; this allows for not constantly re-fetching data
130+
-- for pools that are known to have no metadata.
131+
132+
, getCachedMetadata
133+
:: PoolId -> m (Maybe (Maybe StakePoolOffChainMetadata, UTCTime))
134+
-- ^ Action for fetching the last modification time of a cached result.
135+
-- 'Nothing' is expected when there's no cached result.
81136
}
137+
138+
-- | Create a new HTTP 'Client' in IO with caching support.
139+
newClient
140+
:: Tracer IO MetadataRegistryLog
141+
-> ClientConfig
142+
-> ClientCallbacks IO
143+
-> Client Api IO
144+
newClient tr ClientConfig{manager,baseUrl,cacheTTL} callbacks =
145+
Client { getStakePoolMetadata }
82146
where
83147
run :: ClientM a -> IO (Either ClientError a)
84-
run query = runClientM query (mkClientEnv mgr baseUrl)
148+
run query = runClientM query (mkClientEnv manager baseUrl)
85149

86-
getMetadata =
150+
getFromServer =
87151
client (Proxy @Api)
152+
153+
ClientCallbacks{getCachedMetadata,saveMetadata} =
154+
callbacks
155+
156+
getStakePoolMetadata
157+
:: PoolId
158+
-> IO (Maybe StakePoolOffChainMetadata)
159+
getStakePoolMetadata pid = do
160+
now <- getCurrentTime
161+
getCachedMetadata pid >>= \case
162+
Just (meta, time) | diffUTCTime now time < cacheTTL -> do
163+
traceWith tr $ MsgUsingCached pid time
164+
pure meta
165+
166+
_expiredOrNotCached ->
167+
(handleRequest <$> run (getFromServer (ApiT pid))) >>= \case
168+
Right meta -> do
169+
traceWith tr $ MsgRefreshingMetadata pid (meta, now)
170+
saveMetadata pid (meta, now)
171+
pure meta
172+
Left e -> do
173+
traceWith tr $ MsgUnexpectedError e
174+
pure Nothing
175+
where
176+
handleRequest = \case
177+
Right (ApiT meta) ->
178+
Right (Just meta)
179+
Left (FailureResponse _ res) | responseStatusCode res == status404 ->
180+
Right Nothing
181+
Left e ->
182+
Left e
183+
184+
-- | Capture log events for the Client.
185+
data MetadataRegistryLog
186+
= MsgUsingCached PoolId UTCTime
187+
| MsgRefreshingMetadata PoolId (Maybe StakePoolOffChainMetadata, UTCTime)
188+
| MsgUnexpectedError ClientError
189+
deriving (Generic, Show, Eq)
190+
191+
instance HasPrivacyAnnotation MetadataRegistryLog
192+
instance HasSeverityAnnotation MetadataRegistryLog where
193+
getSeverityAnnotation = \case
194+
MsgUsingCached{} -> Debug
195+
MsgRefreshingMetadata{} -> Debug
196+
MsgUnexpectedError{} -> Warning
197+
198+
instance ToText MetadataRegistryLog where
199+
toText = \case
200+
MsgUsingCached pid time -> T.unwords
201+
[ "Using cached result for"
202+
, pretty pid
203+
, "last modified at"
204+
, T.pack (show time)
205+
]
206+
MsgRefreshingMetadata pid (meta, time) -> T.unwords
207+
[ "Setting metadata for "
208+
, pretty pid
209+
, "="
210+
, maybe "ø" (T.pack . show) meta
211+
, ", last modified at"
212+
, T.pack (show time)
213+
]
214+
MsgUnexpectedError e -> T.unwords
215+
[ "Unexpected error from the aggregation server:"
216+
, T.pack (show e)
217+
]

0 commit comments

Comments
 (0)