OpenAPI specs for API #235
Conversation
Use terms "Read Requests" and "Private API" which are used by the service documentation [1] [1]: https://exonum.com/doc/architecture/services/
OpenAPI does not provide any way to customize model descriptions in responses. We are going to use this approach to ensure that model schema is rigorous enough with accurate enough descriptions.
viktorbezhenar
requested review from
stanislav-tkach,
MartaDubno and
Maria-Nosyk
|
Looks good. I found two technical issues in the UI part of the editor, not sure if its the problem with the code or the editor. The issues are shown on the screenshot below.
|
|
API spec will need an update once exonum/exonum#839 is merged. |
| content: | ||
| application/json: | ||
| schema: | ||
| $ref: "#/components/schemas/BtcAddress" |
There was a problem hiding this comment.
Should probably have nullable property on.
| required: true | ||
| description: Identifier of the validator to query for. | ||
| schema: | ||
| type: integer |
There was a problem hiding this comment.
Could make sense to specify minimum / maximum or format properties here.
| components: | ||
| schemas: | ||
| Hash: | ||
| type: string |
There was a problem hiding this comment.
It could make sense to specify a pattern here (e.g., /[0-9a-f]{64}/i).
| version: 0.9.0 | ||
| description: > | ||
| All explorer endpoints are public. | ||
| `enable_blockchain_explorer` local configuration parameter |
There was a problem hiding this comment.
It could make sense to link to the "Configuration" article in the documentation.
| Hexadecimal string representing a signature (128 hex digits). | ||
| Time: | ||
| type: string | ||
| example: "2018-05-17T10:45:56.057753Z" |
There was a problem hiding this comment.
It could make sense to specify format: dateTime here as per spec.
| type: array | ||
| description: The list of the transactions included into the block. | ||
| items: | ||
| $ref: "#/components/schemas/SerializedTransaction" |
There was a problem hiding this comment.
I think the transaction hashes are used here.
There was a problem hiding this comment.
Yeah, my bad. Thanks for having hawk eyes.
This object is actually serialized from this struct:
pub struct BlockInfo {
pub block: Block,
pub precommits: Vec<Precommit>,
pub txs: Vec<Hash>,
}| - committed | ||
| - in-pool | ||
| - unknown | ||
| TransactionStatus: |
There was a problem hiding this comment.
Maybe, it could make more sense to represent the status as oneOf with a discriminator?
There was a problem hiding this comment.
I'm not sure. It does make sense from the rigorous point of view, but I kinda doubt that we'll get any merit from it.
oneOf does not seem to allow to specify which discriminator values choose which variants. It is a way to join two very different schemas into one. It is used for KnownTransaction here (and could have been used for UnknownTransaction as well), but that's because committed and uncommitted transactions have very different representations.
In this case we still have a single TransactionStatus type with a single purpose. The most common way of handling it should be check the type field for "success" and if it's not it then propagate information from code and description. I think we can leave it as a single type with some meaningful default values for code and description and don't bother with accurate discriminated union representation.
Though, here's how it's represented in the code:
enum TxStatus<'a> {
Success,
Panic { description: &'a str },
Error { code: u8, description: &'a str },
}so I guess we should translate that directly into oneOf. Maybe some day we'll have OpenAPI docs generated from the code. If I were the generator I would translate that enum as oneOf.
|
|
||
| components: | ||
| schemas: | ||
| NullObject: |
There was a problem hiding this comment.
Looks clunky, but I'm not sure if a better type description is possible (e.g., type: "null" doesn't seem to work).
There was a problem hiding this comment.
Yeah. For some reason OpenAPI does not have type: null and this seems to be the closest we can get to 'only null is allowed here'. Though, technically this type specification allows any object in addition to null.
| tags: [Private API] | ||
| summary: Get timestamps of all validators | ||
| description: > | ||
| Returns the latest timestamps indicated by all validator nodes for which time is known. |
There was a problem hiding this comment.
Nit: I'd elaborate that by all validator nodes we mean both current and past ones.
Specify with regular expressions and format constants types of most of string fields.
Usually these are u32 and u64 in Rust code. Set limits accordingly. OpenAPI's minimum and maximum are inclusive. For usize in Rust code specify just the minimum value as the top limit is undefined (though it's most likely the same as for u64). Introduce new model types as appropriate.
It has been removed in 0.9.0. I missed it initially.
Do not duplicate protocol message fields, share them.
"txs" field actually contains list of hashes of transactions, not transactions themselves. It is serialized from BlockInfo struct in src/api/node/public/explorer.rs.
|
Added a bunch of changes to resolve issues pointed out by @slowli. One thing that bothers me is the minimum: 0
maximum: 18446744073709551615 # <== 2^64 - 1However, when rendered by Swagger the limit it shown as 18446744073709552000. I guess Swagger can't handle unsigned 64-bit values accurately. It's not possible to specify the limit as a string, it has to be a number. |
|
@MartaDubno Uncollapsible model entries seem to be a bug in Swagger's editor. As for the examples... Do we need to keep accurate examples in data model sections? They do not seem to word-wrap correctly. Though JSON objects in request examples wrap normally. I guess we can remove the model value examples and keep them only in requests. |
Do not use excessively long string in model value examples as they are not rendered with word-wrapping by Swagger. Leave examples only for primitive values, move all 'big' objects to requests/responses.
|
Can we merge this please? |
There was a problem hiding this comment.
Couple of comments as of 22975a5. Otherwise, looks good. I must say that with the planned transition to the conventional serialization format the spec may become obsolete; but this most probably will take at least several months.
| schema: | ||
| type: integer | ||
| minimum: 0 | ||
| maximum: 4294967295 |
There was a problem hiding this comment.
Validator is represented by u16, so the upper limit should be 65535.
| schema: | ||
| type: string | ||
| nullable: true | ||
| format: "^[0-9a-f]+$" |
There was a problem hiding this comment.
Are you sure this works? I was under the impression (reading the OpenAPI spec) that ECMA-262 regexps should be used (i.e., ones used in JavaScript; something like /^[0-9a-f]+$/i). Additionally, shouldn't the property be pattern, rather than format?
| tx_count: | ||
| type: integer | ||
| minimum: 0 | ||
| maximum: 4294967295 |
There was a problem hiding this comment.
I don't think it makes sense to include u32 or u64 upper restrictions, tbh, especially for returned types. Additionally, u64::max_value() isn't even represented properly in the Swagger doc viewer.
This is a preview version of OpenAPI (aka Swagger) specification of node API extracted from current documentation and source code. This machine-readable format is going to replace the current informal specification of the API in documentation.
It covers the following services:
The content is rather crude, the descriptions are not fully complete, there are some spelling/grammar issues, some examples are missing. But it should be technically accurate for Exonum 0.9.0 (as in describing all endpoints, parameters, and results).
The API is currently not rendered anywhere. You can get a preview with online OpenAPI editor, just paste raw YAML content there. Please refer to those previews and provide feedback on what could be improved from technical standpoint.