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

[CO-322] Fix Swagger names in the spec #3595

Merged
merged 2 commits into from
Sep 14, 2018
Merged

[CO-322] Fix Swagger names in the spec #3595

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b4f933
[CO-322] Introduce test case
parsonsmatt Sep 13, 2018
c8a6170
[CO-322] Fix the issue by replacing < with - and [] with __
parsonsmatt Sep 13, 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
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@

- Improve type safety (and as a consequence, API documentation) of account indexes (CBR-306)

- The Swagger specification had names with illegal characters. These names
where changed to be URL friendly. [PR #3595](https://github.com/input-output-hk/cardano-sl/pull/3595)

### Improvements

- Friendly error mistakes from deserializing invalid addresses instead of brutal 500 (CBR-283)
Expand Down
2 changes: 2 additions & 0 deletions pkgs/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17704,6 +17704,7 @@ license = stdenv.lib.licenses.mit;
, http-client
, http-client-tls
, http-types
, insert-ordered-containers
, ixset-typed
, lens
, memory
Expand Down Expand Up @@ -17924,6 +17925,7 @@ formatting
hedgehog
hspec
hspec-core
insert-ordered-containers
lens
mtl
normaldistribution
Expand Down
2 changes: 2 additions & 0 deletions wallet-new/cardano-sl-wallet-new.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -601,6 +601,7 @@ test-suite wallet-new-specs
, cardano-sl-core-test
, cardano-sl-crypto
, cardano-sl-chain
, cardano-sl-util
, cardano-sl-util-test
, cardano-sl-wallet
, cardano-sl-wallet-new
Expand All @@ -611,6 +612,7 @@ test-suite wallet-new-specs
, formatting
, hedgehog
, hspec
, insert-ordered-containers
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need this import right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I always try to use Control.Lens.At instead of bringing in this dep :/ ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Or Control.Lens.Indexed in that case actually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I needed to get access to the keys of the map and couldn't figure out how to do that with lens :\

, lens
, QuickCheck
, quickcheck-instances
Expand Down
7 changes: 5 additions & 2 deletions wallet-new/src/Cardano/Wallet/API/Response.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,14 @@ instance ToJSON a => MimeRender OctetStream (WalletResponse a) where
instance (ToSchema a, Typeable a) => ToSchema (WalletResponse a) where
declareNamedSchema _ = do
let a = Proxy @a
tyName = toText . show $ typeRep a
tyName = toText . map sanitize . show $ typeRep a
sanitize c
| c `elem` (":/?#[]@!$&'()*+,;=" :: String) = '_'
| otherwise = c
aRef <- declareSchemaRef a
respRef <- declareSchemaRef (Proxy @ResponseStatus)
metaRef <- declareSchemaRef (Proxy @Metadata)
pure $ NamedSchema (Just $ "WalletResponse<" <> tyName <> ">") $ mempty
pure $ NamedSchema (Just $ "WalletResponse-" <> tyName) $ mempty
& type_ .~ SwaggerObject
& required .~ ["data", "status", "meta"]
& properties .~
Expand Down
25 changes: 23 additions & 2 deletions wallet-new/test/SwaggerSpec.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,26 @@ import Universum

import qualified Prelude

import qualified Data.HashMap.Strict.InsOrd as IOMap
import Data.String.Conv
import Data.Swagger
import Pos.Wallet.Aeson.ClientTypes ()
import Servant.API.ContentTypes
import Servant
import Servant.Swagger.Test ()
import Test.Hspec
import Test.Hspec.QuickCheck
import Test.QuickCheck.Instances ()

import Cardano.Wallet.API (InternalAPI, V1API)
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.Core.Update (ApplicationName (..), SoftwareVersion (..))
import Pos.Util.CompileInfo (CompileTimeInfo (CompileTimeInfo),
gitRev)

-- for vendored code
import Data.Aeson (ToJSON)
Expand Down Expand Up @@ -47,10 +53,25 @@ instance ToSchema NoContent where
& maxLength .~ Just 0

spec :: Spec
spec = modifyMaxSuccess (const 10) $
spec = modifyMaxSuccess (const 10) $ do
describe "Swagger Integration" $ do
parallel $ describe "(V1) ToJSON matches ToSchema" $
validateEveryToJSON' (Proxy @ V1.API)
describe "Swagger Validity" $ do
it "has valid URL-compatible names" $ do
let details =
( CompileTimeInfo gitRev
, SoftwareVersion (ApplicationName "cardano-sl") 1
)
swagger = Swagger.api details v1API' Swagger.highLevelDescription
v1API' = Proxy :: Proxy (V1API :<|> InternalAPI)
for_
(IOMap.keys (swagger ^. definitions))
(`shouldSatisfy` noReservedCharacters)

noReservedCharacters :: Text -> Bool
noReservedCharacters =
all (`notElem` (":/?#[]@!$&'()*+,;=" :: [Char]))

-- vendored
validateEveryToJSON'
Expand Down