Translate the Swagger API specification into a Servant API specification.

[jonathanknowles]

Context

We intend to maintain both a Swagger API specification and a Servant API specification, and to keep the two in synchronization with one another.

Task

Translate the Swagger API specification (specifications/api/swagger.yaml) into an equivalent Servant API specification, including:

• Types
• Endpoints

Acceptance Criteria

1. Our Servant API must satisfy the Swagger API specification. Clients generated by the official Swagger Codegen tool should be able to connect and make API calls without issues.
2. Endpoints defined in the Servant API may include specifications of errors that may be thrown.

Development Plan

• [x] Code up a small portion of the API, with at least one endpoint, and all the types required to support that endpoint.
• [x] Extend the API to support all endpoints listed below: (see QA section)
• [x] Translate the postWallet and putWallet operations to the Servant API, along with all associated types.

PR

PR	Base
#76	master
#107	master
#108	master
#119	master
#123	master
#124	master
#125	master
#128	master

QA

• The scope of this ticket has been reduced down to the endpoints discussed during the last iteration planning meeting:

  • List wallet
  • Get wallet
  • Post wallet (- Delete wallet) done as bonus here

• Transaction creation and listing will be covered by #93

• Translated types are available in the haddock documentation here: https://input-output-hk.github.io/cardano-wallet/haddock/cardano-wallet-2.0.0/Cardano-Wallet-Api-Types.html. The strategy here was to re-use most of our low-level types and wrap them in a polymorphic ApiT to define the various serialization instances we needed. This way, we keep the API layers fairly separated from the rest and we avoid cluttering the core code with API-related concerns.

• For now, there's no server actually implementing these types, but we have been already able to perform quite some extensive testing (cf: #91). Testing can be found in Cardano.Wallet.Api.TypesSpec and are of four kinds:

  • Roundtrip tests for JSON for types appearing in the API

  • Golden tests for JSON representation of types appearing the API (prevent breaking change), cf #91

  • Comparison of every response & body param type of the API against their corresponding swagger representation (so here, Wallet, [Wallet] and WalletPostData). It works by generating arbitrary samples for each types, and then, checks whether they satisfy their corresponding schema from the spec. So, it was possible to represent a value in Haskell that would violate one of the constraints we have in swagger, the test would fail.

  • And finally, tests which check that all paths present in our API definition also exist and match one from the specification. We do not do the other direction (verifying that every path in the spec exist in our API) because we obviously haven't implemented everything yet, though in practice, it would be a nice one to have later.

• Added a few negative test cases to test failing paths in various decoders that aren't fully generic (see: #128)

[jonathanknowles]

We also need to think about how we'll test that the Servant API is equivalent to the Swagger API specification.

[KtorZ]

@jonathanknowles On that last remark, I'd recommend looking into:

https://hackage.haskell.org/package/servant-swagger-1.1.7/docs/Servant-Swagger-Test.html#v:validateEveryToJSON

with some inspiration from (https://github.com/input-output-hk/cardano-wallet-legacy/blob/develop/test/unit/API/SwaggerSpec.hs)

And maybe later, give a shot to https://hackage.haskell.org/package/servant-quickcheck-0.0.2.2/docs/Servant-QuickCheck.html later on might be a good idea.

[KtorZ]

@jonathanknowles let's tackle generation of golden tests in a separate ticket (I'll explain tomorrow in the planning meeting).

[piotr-iohk]

@jonathanknowles, @KtorZ - really nice sets of tests for this. I like :+1:

Looking at coveralls there are few places where some cases could be produced:

• https://coveralls.io/builds/22429878/source?filename=src/Cardano/Wallet/Api/Types.hs#L207
• https://coveralls.io/builds/22429878/source?filename=src/Cardano/Wallet/Api/Types.hs#L236
• https://coveralls.io/builds/22429878/source?filename=src/Cardano/Wallet/Api/Types.hs#L280

What do you think?

[KtorZ]

I admit I don't full understand the third one :thinking: ... But I'll look into them all :+1:

[piotr-iohk]

Also this one seems weird -> https://coveralls.io/builds/22429878/source?filename=src/Cardano/Wallet/Api/Types.hs#L388 since eitherToParser is used in other places (e.g. https://coveralls.io/builds/22429878/source?filename=src/Cardano/Wallet/Api/Types.hs#L279).

Anyway, I'll put back to in progress for the time being.

[KtorZ]

For eitherToParser, I suspect lazyness is at play here. The other one seems to be an unused option from our serialization option. Will remove.

[piotr-iohk]

lgtm :+1: