Utoipa for the solvers crate #2668

squadgazzz · 2024-04-26T18:53:25Z

Description

Initial step to the automated OpenAPI schema generation. The implementation strictly follows the current manually created OpenAPI schema to avoid breaking changes. That is the main reason why some of the structs have manually implemented ToSchema trait.

Changes

crates/solvers/openapi.yml is now generated using cargo build
This is possible because of a new crate openapi-generator which implements a workaround found by community
pull-request CI job is modified and now fails if the schema changes weren't included in the commit

How to review

crates/solver/openapi.yml file can be compared with the previous state: https://github.com/cowprotocol/services/blob/03748ae9d04456f34c71751926c861357dc1018d/crates/solvers/openapi.yml

How to test

Existing tests + staging.

Related Issues

Partly fixes #2203

Further steps

Some of the schemas are implemented manually. This should be fixed in a separate PR since the OpenAPI schema will change, this should only be done with structs that are used internally to avoid breaking changes.
Integrate the same into the gnosis/solvers repo
Configuration can be simplified with the help of utoipauto, but currently, it is not really stable(#[derive(utoipa::ToSchema)] detection fails ProbablyClem/utoipauto#23)

# Conflicts: # crates/solvers-dto/src/auction.rs

squadgazzz · 2024-04-28T09:37:44Z

crates/solvers/openapi.yml

+      - preInteractions
+      - postInteractions
+      - sellTokenSource
+      - buyTokenDestination


This is new because we forgot to alter it in the #2620

squadgazzz · 2024-04-28T09:44:34Z

crates/solvers-dto/src/common.rs

@@ -0,0 +1,37 @@
+use utoipa::ToSchema;
+
+// Structs for the utoipa OpenAPI schema generator.


These structs should be used directly in the code. Worth a separate PR.

fleupold

This looks very promising!

fleupold · 2024-04-29T07:33:11Z

crates/solvers-dto/src/auction.rs

 #[serde(rename_all = "lowercase")]
-pub enum SigningScheme {
+pub enum LegacySigningScheme {


It looks like these two enums are exactly the same. Can we not combine them? Why is one considered more legacy than the other?

Because from the aution side we have lowercased variants while from the solution - camelCase.

services/crates/solvers-dto/src/auction.rs

Lines 139 to 141 in e469368

// todo: There is a conflict between solution's SigningScheme which is in

// camelCase. There is no way to keep 2 object with the same name in the OpenAPI

// schema. Temporarily renamed the struct. Must be migrated to the camelCase.

fleupold · 2024-04-29T07:33:56Z

crates/solvers-dto/src/auction.rs

+// todo: Currently, it strictly follows the manual api schema. This has to be
+// automated and deleted.


Is this something blocking this PR or just future work to make working with this code more ergonomic?

This PR aims to automate the OpenAPI schema generation without altering its structure. In separate PRs, it would be good to try to remove manual ToSchema implementations, but this will change the schema structures.

crates/solvers/src/tests/mod.rs

crates/solvers/src/api/routes/solve/mod.rs

MartinquaXD

Nice work. I hope we can use this initiative to introduce commonly shared structs across the different crates. I.e. we probably don't have to have a TokenAmount struct in every binary.

crates/solvers-dto/src/notification.rs

crates/solvers-dto/src/solution.rs

MartinquaXD · 2024-04-29T12:05:47Z

crates/solvers/src/api/mod.rs

@@ -46,3 +47,74 @@ impl Api {
        server.with_graceful_shutdown(shutdown).await
    }
 }
+
+// migrate to utoipauto once the issue is solved https://github.com/ProbablyClem/utoipauto/issues/23
+pub fn generate_openapi_yaml() -> Result<String, serde_yaml::Error> {


Seems a bit backwards to generate this from the solvers repo. From my view the driver is the one dictating the interface so it makes more sense to me to derive the openapi spec from it's requests and expected responses.

gnosis/solvers will generate the yaml file from the same crate. IMO, that should live where the API server with all the endpoints is declared.

MartinquaXD

I plugged in the generated yaml file into a openapi viewer and a bunch of things get displayed weird:

token addresses (and other hex strings) get interpreted as floats
liquidity entries have a bunch of weird properties in the examples:

      "tokens": {
        "additionalProp1": {
          "balance": "1234567890",
          "scalingFactor": "13.37",
          "weight": "13.37"
        },
        ...

Probably makes sense to tweak the PR and sanity check with https://editor.swagger.io/ until things look good.

squadgazzz · 2024-04-30T16:05:39Z

Probably makes sense to tweak the PR and sanity check with editor.swagger.io until things look good.

Found the issue with HEX values: juhaku/utoipa#875. The fastest way would be provide examples with strings length up to 35.

liquidity entries have a bunch of weird properties in the examples:

The original yaml contains the same issue. I can try to fix it in a separate PR.

squadgazzz · 2024-05-01T10:30:52Z

Temporarily shortened the example values to get the quotes back: 71f1eee.
Will try to fix it either in the serde_yaml or utoipa lib.

~~Another option is to switch to JSON -> #2683~~

Switched to JSON.

github-actions · 2024-05-10T00:06:55Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions · 2024-05-21T00:41:20Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions · 2024-05-29T00:07:43Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

m-lord-renkse · 2024-05-29T07:13:58Z

crates/solvers-dto/src/auction.rs

    #[serde_as(as = "Option<DisplayFromStr>")]
+    #[schema(value_type = String)]


is this correct to be type String? shouldn't it be integer?

m-lord-renkse · 2024-05-29T07:14:43Z

crates/solvers-dto/src/auction.rs

    pub id: Option<i64>,
-    pub tokens: HashMap<H160, Token>,
+    /// A map of token addresses to token information.
+    pub tokens: HashMap<H160, TokenInfo>,


isn't there a way to add the value type also the token? the H160 is type address 🤔

m-lord-renkse · 2024-05-29T07:15:54Z

crates/solvers-dto/src/auction.rs

    pub buy_token: H160,
+    #[schema(value_type = TokenAmount)]


what do you think of creating a "Deserializable" type called TokenAmount (if it doesn't exist already), derive the proper schema to the new type, and use the type all over the deserializable structs?

m-lord-renkse · 2024-05-29T07:17:38Z

crates/solvers-dto/src/auction.rs

@@ -200,6 +426,52 @@ pub struct WeightedProductPool {
    pub version: WeightedProductVersion,
 }

+impl ToSchema<'static> for WeightedProductPool {


why this can be directly derived in the #[derive()]?

m-lord-renkse · 2024-05-29T07:17:58Z

crates/solvers-dto/src/auction.rs

@@ -178,9 +373,40 @@ pub struct ConstantProductPool {
    pub fee: BigDecimal,
 }

+impl ToSchema<'static> for ConstantProductPool {


why this can be directly derived in the #[derive()]?

m-lord-renkse · 2024-05-29T07:41:06Z

crates/solvers-dto/src/common.rs

+#[schema(
+    example = "0x0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000"
+)]
+#[allow(dead_code)]


why all of this dead code?

github-actions · 2024-06-06T00:06:48Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions · 2024-06-14T00:06:36Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions · 2024-06-22T00:06:51Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions · 2024-07-02T00:07:06Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions · 2024-07-10T00:07:32Z

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

squadgazzz added 9 commits April 26, 2024 18:17

Utoipa for the solvers crate

5b156c5

Override existing file

4e3aff0

Fail on the uncommited changes

7adcc21

Formatting

b5dc2a5

Comment

90f3385

Flatten

60b1cea

Revert some changes

9c54fbc

Comments

c50ac2a

License

5029ea0

squadgazzz marked this pull request as ready for review April 26, 2024 19:00

squadgazzz requested a review from a team as a code owner April 26, 2024 19:00

squadgazzz added 3 commits April 26, 2024 20:11

Error message

cd3a5a6

Drop utoipauto for now

2ce5b3c

Merge branch 'refs/heads/main' into utoipa/solvers

148073a

# Conflicts: # crates/solvers-dto/src/auction.rs

squadgazzz force-pushed the utoipa/solvers branch from f3896c8 to e469368 Compare April 28, 2024 09:35

Fixes after merge

e469368

squadgazzz commented Apr 28, 2024

View reviewed changes

Dependency fix

e791b27

squadgazzz commented Apr 28, 2024

View reviewed changes

fleupold reviewed Apr 29, 2024

View reviewed changes

Redundant changes

5264460

MartinquaXD reviewed Apr 29, 2024

View reviewed changes

squadgazzz added 2 commits April 29, 2024 15:56

Drop duplicated rust docs from structs not deriving ToSchema

9771fea

Minor refactoring

ae40b52

MartinquaXD reviewed Apr 30, 2024

View reviewed changes

Shorten examples

71f1eee

squadgazzz added 2 commits May 1, 2024 11:32

Merge branch 'main' into utoipa/solvers

b7f3b45

Solvers JSON OpenAPI schema (#2683)

af43019

squadgazzz added 2 commits May 2, 2024 19:27

Merge branch 'main' into utoipa/solvers

1249963

Fixed extensions

53d4bc2

github-actions bot added the stale label May 10, 2024

squadgazzz removed the stale label May 13, 2024

github-actions bot added the stale label May 21, 2024

squadgazzz removed the stale label May 21, 2024

github-actions bot added the stale label May 29, 2024

m-lord-renkse reviewed May 29, 2024

View reviewed changes

squadgazzz removed the stale label May 29, 2024

squadgazzz marked this pull request as draft May 29, 2024 09:23

github-actions bot added the stale label Jun 6, 2024

squadgazzz removed the stale label Jun 6, 2024

github-actions bot added the stale label Jun 14, 2024

squadgazzz removed the stale label Jun 14, 2024

github-actions bot added the stale label Jun 22, 2024

squadgazzz removed the stale label Jun 24, 2024

github-actions bot added the stale label Jul 2, 2024

squadgazzz removed the stale label Jul 2, 2024

github-actions bot added the stale label Jul 10, 2024

squadgazzz added blocked This issue is blocked by some other work and removed stale labels Jul 10, 2024

fleupold mentioned this pull request Aug 9, 2024

fix: lowercase orderbook CompetitionOrderStatus types to match real output #2858

Merged

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Utoipa for the solvers crate #2668

Utoipa for the solvers crate #2668

squadgazzz commented Apr 26, 2024 •

edited

Loading

squadgazzz Apr 28, 2024

squadgazzz Apr 28, 2024

fleupold left a comment

fleupold Apr 29, 2024

squadgazzz Apr 29, 2024 •

edited

Loading

fleupold Apr 29, 2024

squadgazzz Apr 29, 2024

MartinquaXD left a comment

MartinquaXD Apr 29, 2024

squadgazzz Apr 29, 2024

MartinquaXD left a comment

squadgazzz commented Apr 30, 2024 •

edited

Loading

squadgazzz commented May 1, 2024 •

edited

Loading

github-actions bot commented May 10, 2024

github-actions bot commented May 21, 2024

github-actions bot commented May 29, 2024

m-lord-renkse May 29, 2024

m-lord-renkse May 29, 2024

m-lord-renkse May 29, 2024

m-lord-renkse May 29, 2024

m-lord-renkse May 29, 2024

m-lord-renkse May 29, 2024

github-actions bot commented Jun 6, 2024

github-actions bot commented Jun 14, 2024

github-actions bot commented Jun 22, 2024

github-actions bot commented Jul 2, 2024

github-actions bot commented Jul 10, 2024

		@@ -0,0 +1,37 @@
		use utoipa::ToSchema;

		// Structs for the utoipa OpenAPI schema generator.

	// todo: There is a conflict between solution's SigningScheme which is in
	// camelCase. There is no way to keep 2 object with the same name in the OpenAPI
	// schema. Temporarily renamed the struct. Must be migrated to the camelCase.

		// todo: Currently, it strictly follows the manual api schema. This has to be
		// automated and deleted.

		#[serde_as(as = "Option<DisplayFromStr>")]
		#[schema(value_type = String)]

Utoipa for the solvers crate #2668

Are you sure you want to change the base?

Utoipa for the solvers crate #2668

Conversation

squadgazzz commented Apr 26, 2024 • edited Loading

Description

Changes

How to review

How to test

Related Issues

Further steps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

fleupold left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

squadgazzz Apr 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinquaXD left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinquaXD left a comment

Choose a reason for hiding this comment

squadgazzz commented Apr 30, 2024 • edited Loading

squadgazzz commented May 1, 2024 • edited Loading

github-actions bot commented May 10, 2024

github-actions bot commented May 21, 2024

github-actions bot commented May 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jun 6, 2024

github-actions bot commented Jun 14, 2024

github-actions bot commented Jun 22, 2024

github-actions bot commented Jul 2, 2024

github-actions bot commented Jul 10, 2024

squadgazzz commented Apr 26, 2024 •

edited

Loading

squadgazzz Apr 29, 2024 •

edited

Loading

squadgazzz commented Apr 30, 2024 •

edited

Loading

squadgazzz commented May 1, 2024 •

edited

Loading