Skip to content
This repository was archived by the owner on Aug 18, 2020. It is now read-only.

[CO-413] Removing wallet-new/src/Cardano/Wallet/Orphans* #3707

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
af27e67
Merge pull request #3667 from input-output-hk/KtorZ/CO-394/remove-leg…
KtorZ Sep 27, 2018
4511aad
[CO-373] Copy Mnemonic.hs to wallet-new as BIP39.hs
Anviking Sep 24, 2018
876650e
[CO-373] Update BIP39Spec
Anviking Sep 25, 2018
08efbe2
[CO-373] Remove tools/dbgen executable and lib
Anviking Sep 27, 2018
f48828a
[CO-403] Remove the reference to the old wallet
parsonsmatt Sep 28, 2018
9e7afa8
Merge pull request #3663 from input-output-hk/anviking/CO-373/move-mn…
KtorZ Sep 28, 2018
263a2d0
[CO-405] Remove OldStorage instances
parsonsmatt Sep 28, 2018
bd5f7db
[CO-404] Remove old wallet types from Example
parsonsmatt Sep 28, 2018
a3c5ad3
Merge pull request #3686 from input-output-hk/matt/co-403/remove-wall…
KtorZ Oct 1, 2018
3ac8fff
[CO-405] Remove old instances module
parsonsmatt Sep 28, 2018
a7aa43f
Merge pull request #3687 from input-output-hk/matt/co-404/example
KtorZ Oct 1, 2018
0422acf
[CO-406] Remove swagger import
parsonsmatt Sep 28, 2018
89dbc08
Merge pull request #3688 from input-output-hk/matt/co-405/remove-from…
KtorZ Oct 1, 2018
1eda5e6
Merge pull request #3689 from input-output-hk/matt/co-406/swagger
KtorZ Oct 1, 2018
62b01e3
[CO-402] Remove dependency on `Pos.Wallet.Web.Tracking.Decrypt`
Anviking Sep 28, 2018
f10719e
[CO-402] Remove partial functions
Anviking Oct 1, 2018
f3d7bf3
[CO-380] Removing Migration.hs Migration/Types.hs and Migration/Monad…
paweljakubas Oct 3, 2018
5a7b414
Merge branch 'anviking/CO-402/no-wallet-in-decrypt' into CO-372/TheGr…
KtorZ Oct 2, 2018
5f7f8b6
[CO-408] Test keyToWalletDecrCredentials
Anviking Oct 2, 2018
d6825e5
Merge remote-tracking branch 'origin/paweljakubas/CO-380/remove-migra…
KtorZ Oct 3, 2018
ebde06f
Merge pull request #3699 from input-output-hk/anviking/CO-408/decrypt…
KtorZ Oct 4, 2018
8c4fc47
[CO-413] Removing wallet-new/src/Cardano/Wallet/Orphans*
paweljakubas Oct 4, 2018
366da28
Merge branch 'CO-372/TheGreatCleanup' into paweljakubas/CO-413/remove…
paweljakubas Oct 10, 2018
bf33d9f
[CO-413] Add Test.Pos.Chain.Update.Arbitrary import
paweljakubas Oct 10, 2018
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
4 changes: 0 additions & 4 deletions wallet-new/cardano-sl-wallet-new.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -116,10 +116,6 @@ library
Cardano.Wallet.Kernel.Util.StrictNonEmpty
Cardano.Wallet.Kernel.Util.StrictStateT
Cardano.Wallet.Kernel.Wallets
Cardano.Wallet.Orphans
Cardano.Wallet.Orphans.Aeson
Cardano.Wallet.Orphans.Arbitrary
Cardano.Wallet.Orphans.Bi
Cardano.Wallet.Server
Cardano.Wallet.Server.CLI
Cardano.Wallet.Server.Middlewares
Expand Down
3 changes: 1 addition & 2 deletions wallet-new/src/Cardano/Wallet/API/V1/Swagger/Example.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import Data.Swagger (Definitions, NamedSchema (..), Schema,
import Data.Swagger.Declare (Declare)
import Data.Typeable (typeOf)

import Cardano.Wallet.Orphans.Arbitrary ()
import Test.QuickCheck (Arbitrary (..), listOf1)
import Test.QuickCheck.Gen (Gen (..), resize)
import Test.QuickCheck.Random (mkQCGen)
Expand All @@ -21,7 +20,7 @@ class Arbitrary a => Example a where
example = arbitrary

instance Example ()
instance Example a => Example (NonEmpty a)
instance (Example a, Arbitrary (NonEmpty a) ) => Example (NonEmpty a)

-- NOTE: we don't want to see empty list examples in our swagger doc :)
instance Example a => Example [a] where
Expand Down
51 changes: 40 additions & 11 deletions wallet-new/src/Cardano/Wallet/API/V1/Types.hs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
{-# LANGUAGE StandaloneDeriving #-}
{-# LANGUAGE StrictData #-}
{-# LANGUAGE TemplateHaskell #-}

{-# LANGUAGE ViewPatterns #-}
-- The hlint parser fails on the `pattern` function, so we disable the
-- language extension here.
{-# LANGUAGE NoPatternSynonyms #-}
Expand Down Expand Up @@ -161,8 +161,9 @@ import Data.Swagger.Internal.TypeShape (GenericHasSimpleShape,
GenericShape)
import Data.Text (Text, dropEnd, toLower)
import qualified Data.Text as T
import Data.Version (Version)
import Formatting (bprint, build, fconst, int, sformat, stext, (%))
import Data.Version (Version (..), parseVersion, showVersion)
import Formatting (bprint, build, fconst, int, sformat, shown, stext,
(%))
import qualified Formatting.Buildable
import Generics.SOP.TH (deriveGeneric)
import GHC.Generics (Generic, Rep)
Expand All @@ -186,7 +187,6 @@ import Cardano.Wallet.API.V1.Generic (jsendErrorGenericParseJSON,
jsendErrorGenericToJSON)
import Cardano.Wallet.API.V1.Swagger.Example (Example, example,
genExample)
import Cardano.Wallet.Orphans.Aeson ()
import Cardano.Wallet.Types.UtxoStatistics
import Cardano.Wallet.Util (mkJsonKey, showApiUtcTime)

Expand All @@ -195,7 +195,6 @@ import qualified Pos.Binary.Class as Bi
import qualified Pos.Chain.Txp as Txp
import qualified Pos.Chain.Update as Core
import qualified Pos.Client.Txp.Util as Core
import Pos.Core (addressF)
import qualified Pos.Core as Core
import Pos.Crypto (Hash, PublicKey (..), decodeHash, hashHexF)
import qualified Pos.Crypto.Signing as Core
Expand All @@ -206,7 +205,10 @@ import Pos.Infra.Util.LogSafe (BuildableSafeGen (..), SecureLog (..),
buildSafe, buildSafeList, buildSafeMaybe,
deriveSafeBuildable, plainOrSecureF)
import Pos.Util.Servant (Flaggable (..))

import Test.Pos.Chain.Update.Arbitrary ()
import Test.Pos.Core.Arbitrary ()
import Text.ParserCombinators.ReadP (readP_to_S)

-- | Declare generic schema, while documenting properties
-- For instance:
Expand Down Expand Up @@ -312,7 +314,7 @@ instance Bounded a => Bounded (V1 a) where
minBound = V1 $ minBound @a
maxBound = V1 $ maxBound @a

instance Buildable a => Buildable (V1 a) where
instance {-# OVERLAPPABLE #-} Buildable a => Buildable (V1 a) where
build (V1 x) = bprint build x

instance Buildable (SecureLog a) => Buildable (SecureLog (V1 a)) where
Expand All @@ -321,7 +323,6 @@ instance Buildable (SecureLog a) => Buildable (SecureLog (V1 a)) where
instance (Buildable a, Buildable b) => Buildable (a, b) where
build (a, b) = bprint ("("%build%", "%build%")") a b


--
-- Benign instances
--
Expand Down Expand Up @@ -380,8 +381,17 @@ instance ToSchema (V1 Core.Coin) where
& type_ .~ SwaggerNumber
& maximum_ .~ Just (fromIntegral Core.maxCoinVal)

instance ToHttpApiData Core.Coin where
toQueryParam = pretty . Core.coinToInteger

instance FromHttpApiData Core.Coin where
parseUrlPiece p = do
c <- Core.Coin <$> parseQueryParam p
Core.checkCoin c
pure c
Copy link
Contributor

Choose a reason for hiding this comment

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

We gotta need a roundtrip Aeson tests for that one 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

certainly

Copy link
Contributor Author

@paweljakubas paweljakubas Oct 8, 2018

Choose a reason for hiding this comment

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

there is already in wallet-new/test/MarshallingSpec.hs line 95:

httpApiDataRoundtripProp @Core.Coin Proxy

but we are missing this one (although have aesonRoundtripProp @(V1 Core.Coin) Proxy):

aesonRoundtripProp @Core.Coin Proxy

But if we test aeson roundtrips of V1 Something do we need to check aeson roundtrips of Something ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I meant the httpAPIData round trip of course :/ ...

I didn't realize at first this was also (again) an orphan instance. I think we want instances to be on V1 Core.Coin here (like the others above). In such case (as for aeson instances, there's no point of doing a rountrip test on the non-wrapped type (Core.Coin) itself, because we do want to test and use the wrapped instance. And chances are that there's not even an instance defined for the unwrapped type, which is good.

May you do this change 🙏 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

will be addressed in https://iohk.myjetbrains.com/youtrack/issue/CO-422 for reasons specified therein


instance ToJSON (V1 Core.Address) where
toJSON (V1 c) = String $ sformat addressF c
toJSON (V1 c) = String $ sformat Core.addressF c

instance FromJSON (V1 Core.Address) where
parseJSON (String a) = case Core.decodeTextAddress a of
Expand Down Expand Up @@ -2405,17 +2415,36 @@ instance BuildableSafeGen SlotDuration where
data NodeSettings = NodeSettings {
setSlotDuration :: !SlotDuration
, setSoftwareInfo :: !(V1 Core.SoftwareVersion)
, setProjectVersion :: !Version
, setProjectVersion :: !(V1 Version)
, setGitRevision :: !Text
} deriving (Show, Eq, Generic)

#if !(MIN_VERSION_swagger2(2,2,2))
-- See note [Version Orphan]
instance ToSchema Version where
instance ToSchema (V1 Version) where
declareNamedSchema _ =
pure $ NamedSchema (Just "Version") $ mempty
pure $ NamedSchema (Just "V1Version") $ mempty
& type_ .~ SwaggerString

instance Buildable (V1 Version) where
build (V1 v) = bprint shown v

instance Buildable (SecureLog (V1 Version)) where
build (SecureLog x) = Formatting.Buildable.build x

instance ToJSON (V1 Version) where
toJSON (V1 v) = toJSON (showVersion v)

instance FromJSON (V1 Version) where
parseJSON (String v) = case readP_to_S parseVersion (T.unpack v) of
(reverse -> ((ver,_):_)) -> pure (V1 ver)
_ -> mempty
parseJSON x = typeMismatch "Not a valid Version" x
Copy link
Contributor

Choose a reason for hiding this comment

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

What's with the reverse 😕 ?

   parseJSON (String v) = case readP_to_S parseVersion (T.unpack v) of
        [(ver, _)] -> pure (V1 ver)
        _          -> mempty

Isn't that enough ?

Copy link
Contributor Author

@paweljakubas paweljakubas Oct 10, 2018

Choose a reason for hiding this comment

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

Prelude> import Data.Version
Prelude Data.Version> let v = makeVersion [1,2,3]
Prelude Data.Version> v
Version {versionBranch = [1,2,3], versionTags = []}
Prelude Data.Version> import Text.ParserCombinators.ReadP
Prelude Data.Version Text.ParserCombinators.ReadP> let out = readP_to_S parseVersion (showVersion v)
Prelude Data.Version Text.ParserCombinators.ReadP> out
[(Version {versionBranch = [1], versionTags = []},".2.3"),(Version {versionBranch = [1,2], versionTags = []},".3"),(Version {versionBranch = [1,2,3], versionTags = []},"")]

I need the last element. And I can have also [1,2] - hence the need to pattern match against the last element in the list

Copy link
Contributor

Choose a reason for hiding this comment

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

Arf. Okay.


instance Arbitrary (V1 Version) where
arbitrary = fmap V1 arbitrary
Copy link
Contributor

Choose a reason for hiding this comment

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

Where does the Arbitrary Version comes from ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Ho :o !
That's a surprise. Great 🎉



-- Note [Version Orphan]
-- I have opened a PR to add an instance of 'Version' to the swagger2
-- library. When the PR is merged, we can delete the instance here and remove the warning from the file.
Expand Down
1 change: 0 additions & 1 deletion wallet-new/src/Cardano/Wallet/Kernel/DB/Util/AcidState.hs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import Cardano.Wallet.Kernel.DB.Util.IxSet (HasPrimKey, Indexable,
import qualified Cardano.Wallet.Kernel.DB.Util.IxSet as IxSet
import qualified Cardano.Wallet.Kernel.DB.Util.Zoomable as Z
import Cardano.Wallet.Kernel.Util.StrictStateT
import Cardano.Wallet.Orphans ()
import UTxO.Util (mustBeRight)

{-------------------------------------------------------------------------------
Expand Down
19 changes: 0 additions & 19 deletions wallet-new/src/Cardano/Wallet/Orphans.hs

This file was deleted.

11 changes: 0 additions & 11 deletions wallet-new/src/Cardano/Wallet/Orphans/Aeson.hs

This file was deleted.

52 changes: 0 additions & 52 deletions wallet-new/src/Cardano/Wallet/Orphans/Arbitrary.hs

This file was deleted.

3 changes: 0 additions & 3 deletions wallet-new/src/Cardano/Wallet/Orphans/Bi.hs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ getNodeSettings w = liftIO $
V1.NodeSettings
<$> (mkSlotDuration <$> Node.getNextEpochSlotDuration node)
<*> (V1 <$> Node.curSoftwareVersion node)
<*> pure version
<*> pure (V1 version)
<*> (mkGitRevision <$> Node.compileInfo node)
where
mkSlotDuration :: Millisecond -> V1.SlotDuration
Expand Down
1 change: 0 additions & 1 deletion wallet-new/test/Cardano/Wallet/WalletLayer/QuickCheck.hs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ module Cardano.Wallet.WalletLayer.QuickCheck
import Universum

import Cardano.Wallet.Kernel.Diffusion (WalletDiffusion (..))
import Cardano.Wallet.Orphans.Arbitrary ()
import Cardano.Wallet.WalletLayer (ActiveWalletLayer (..),
DeleteAccountError (..), DeleteWalletError (..),
GetAccountError (..), GetAccountsError (..),
Expand Down
5 changes: 3 additions & 2 deletions wallet-new/test/MarshallingSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,12 @@ import Universum
import Control.Lens (from, to)
import Data.Aeson
import qualified Data.ByteString as BS
import Data.SafeCopy hiding (Migrate)
import Data.SafeCopy hiding (Migrate, Version)
import Data.Serialize (runGet, runPut)
import Data.Time (UTCTime (..), fromGregorian)
import Data.Time.Clock.POSIX (POSIXTime)
import Data.Typeable (typeRep)
import Data.Version (Version)
import Pos.Client.Txp.Util (InputSelectionPolicy)
import qualified Pos.Crypto as Crypto
import Servant.API (FromHttpApiData (..), ToHttpApiData (..))
Expand Down Expand Up @@ -43,7 +44,6 @@ import Cardano.Wallet.API.V1.Types
import Cardano.Wallet.Kernel.DB.HdWallet (HdRoot)
import Cardano.Wallet.Kernel.DB.InDb (InDb (..))
import qualified Cardano.Wallet.Kernel.Util.Strict as Strict
import Cardano.Wallet.Orphans ()
import qualified Cardano.Wallet.Util as Util

-- | Tests whether or not some instances (JSON, Bi, etc) roundtrips.
Expand Down Expand Up @@ -84,6 +84,7 @@ spec = parallel $ describe "Marshalling & Unmarshalling" $ do
aesonRoundtripProp @SyncThroughput Proxy
aesonRoundtripProp @AccountIndex Proxy
aesonRoundtripProp @(V1 AddressOwnership) Proxy
aesonRoundtripProp @(V1 Version) Proxy

-- HttpApiData roundtrips
httpApiDataRoundtripProp @AccountIndex Proxy
Expand Down
7 changes: 4 additions & 3 deletions wallet-new/test/SwaggerSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ import Cardano.Wallet.API.Response (ValidJSON)
import qualified Cardano.Wallet.API.V1 as V1
import Cardano.Wallet.API.V1.Swagger ()
import qualified Cardano.Wallet.API.V1.Swagger as Swagger
import Cardano.Wallet.Orphans.Aeson ()
import Cardano.Wallet.Orphans.Arbitrary ()
import Pos.Chain.Update (ApplicationName (..), SoftwareVersion (..))
import Pos.Util.CompileInfo (CompileTimeInfo (CompileTimeInfo),
gitRev)
Expand All @@ -31,7 +29,7 @@ import Pos.Util.CompileInfo (CompileTimeInfo (CompileTimeInfo),
import Data.Aeson (ToJSON)
import Servant.Swagger.Internal.Test (props)
import Servant.Swagger.Internal.TypeLevel (BodyTypes, Every, TMap)
import Test.QuickCheck (Arbitrary)
import Test.QuickCheck (Arbitrary, arbitrary)

-- Syntethic instances and orphans to be able to use `validateEveryToJSON`.
-- In the future, hopefully, we will never need these.
Expand All @@ -52,6 +50,9 @@ instance ToSchema NoContent where
& type_ .~ SwaggerArray
& maxLength .~ Just 0

instance Arbitrary NoContent where
arbitrary = pure NoContent

spec :: Spec
spec = modifyMaxSuccess (const 10) $ do
describe "Swagger Integration" $ do
Expand Down