Parameters as struct implementing IntoParams for warp (and other frameworks) #150

kellpossible · 2022-05-30T04:13:42Z

Support accepting params(Type) for path where Type implements IntoParams, closing Serde Deserialize parameters #129.
Modified todo-warp to show this feature in use.
Implement IntoParams style container attribute #151 container attributes for IntoParams including style and parameter_in. For example: #[param(style = Form, parameter_in = Query)].

TODO:

Some unit tests with this feature.
Figure out what to do about ParameterIn which is currently documented as an "internal trait". Perhaps this trait could also be derived?
Support paths like in Add expr path support for components in openapi #161
~~Fix todo-warp project~~ edit: looks like this is a regression on master todo-warp example broken #173

This work was sponsored by Arctoris.

kellpossible · 2022-05-31T03:53:43Z

Figure out what to do about ParameterIn which is currently documented as an "internal trait". Perhaps this trait could also be derived?

As a solution to this I decided to implement #151 which will allow a container attribute for in to be specified which can optionally be used in place of relying on the ParamaterIn trait.

kellpossible · 2022-05-31T11:49:27Z

I ended up implementing the in attribute slightly differently, instead opting to have a named parameter called parameter_in because it allowed a simpler/cleaner parsing. Perhaps it might be worth a breaking change (or a deprecating change) for the field attribute to make it consistent too, because the current version favours write-ability over readability. From trying to implement parsing for an attribute named in I eventually figured out that in is probably not a valid Ident perhaps this is part of the reason why we ended up with the current implementation?

kellpossible · 2022-05-31T11:50:05Z

@juhaku I think this PR is ready for review

`IntoParams`, implementing juhaku#129

field

as PascalCase instead of lowercase, to match the style parsing.

actix test failure)

kellpossible · 2022-06-16T00:03:55Z

@juhaku I've rebased this to fix conflicts with #156

structs

juhaku · 2022-06-17T00:16:24Z

I ended up implementing the in attribute slightly differently, instead opting to have a named parameter called parameter_in because it allowed a simpler/cleaner parsing. Perhaps it might be worth a breaking change (or a deprecating change) for the field attribute to make it consistent too, because the current version favours write-ability over readability. From trying to implement parsing for an attribute named in I eventually figured out that in is probably not a valid Ident perhaps this is part of the reason why we ended up with the current implementation?

The reason was to have as short syntax as possible but still being clear enough. Indeed I agree these 2 parsing syntaxes could be unified for clarity to the api consumers not having to guess the syntax.

The in is indeed reserved keyword in Rust thus does not play well with naming things directly same. But not sure at the moment whether it could be used like in = Path within the macro syntax. Maybe it could be tried.

I guess the next release will anyway be breaking release since I want to and need to make some changes to the actix-web support so it would work the same way that the type with IntoParams need to be defined in params(MyType) thus allowing users conditionally use params without IntoParams as well.

juhaku

Super, 🥇 This is indeed pretty good base for getting IntoParams available to all the frameworks. Just some minor things to clarify :)

utoipa-gen/src/lib.rs

juhaku · 2022-06-16T23:39:18Z

utoipa-gen/src/path.rs

+/// `#[utoipa::path(params(...))]` attribute.
+#[cfg_attr(feature = "debug", derive(Debug))]
+#[derive(Default)]
+struct Params<'p> {


Just wondering is this extra Params type necessary?

utoipa-gen/src/path.rs

juhaku · 2022-06-16T23:47:48Z

utoipa-gen/src/path/parameter.rs

@@ -27,10 +31,35 @@ use super::property::Property;
 #[cfg_attr(feature = "debug", derive(Debug))]
 pub enum Parameter<'a> {
    Value(ParameterValue<'a>),
+    /// Identifier for a struct that implements `IntoParams` trait.
+    Struct(ExprPath),
    #[cfg(any(feature = "actix_extras", feature = "rocket_extras"))]
    TokenStream(TokenStream),


Here the TokenStream type was actually for that same purpse as now the Struct exists. The TokenStream will be later on used in extension code in similar manner <#struct as utoipa::IntoParams>::into_params() Im just wondering could these two types somehow be combined to avoid less fragmentation?

Is TokenStream a little too permissive for this? I'll admit I haven't taken a proper look at the actix/rocket side of the equation.

For the purpose it is used in acitx-web extras https://github.com/juhaku/utoipa/blob/master/utoipa-gen/src/ext/actix.rs#L67 it is kind of necessary to have it to allow more than a expression path. Note it creates assert trait for the IntoParmas which could also be implemented here as well I think? 🤔

utoipa-gen/src/lib.rs

juhaku · 2022-06-16T23:53:46Z

utoipa-gen/src/path/parameter.rs

+        match &*style.to_string() {
+            "Path" => Ok(Self::Path),
+            "Query" => Ok(Self::Query),
+            "Header" => Ok(Self::Header),
+            "Cookie" => Ok(Self::Cookie),
+            _ => Err(Error::new(style.span(), EXPECTED_STYLE)),


Should we define one way how the parameter_in will be parsed, since now it is parsed with 2 different syntax one here with capitalized first letter, and in tuple syntax all lowercase. It would be better if there was only one way of doing things for clarity to the users.

If we change the other location to CamelCase that will be a breaking change, but it's also easier to recognise it as being one of the enum variants. I agree with you that it should be consistent, which direction would you like to take it?

Considering this library is already 1.0 I guess the aim is to reduce breaking changes where possible, perhaps I'll change this to lower case too?

We can have it Pascal case. It will be breaking change but then it's going to be one. I consider the major release as a "how many mistakes have been done" and it can be increased according to the semver. But for sure it if it is possible to avoid breaking releases then it should be avoided as much as possible. But in this case it is fine for me. Then I can introduce other breaking changes as well with next release. So next release will then be 2.0.0

Okay sounds good I'll try to share the parsing then for this.

@juhaku how does it look now with 451ca01 ?

Super that looks good

utoipa-gen/src/schema/into_params.rs

juhaku · 2022-06-17T00:00:03Z

utoipa-gen/src/schema/into_params.rs

                    help = "Try defining `#[into_params(names(...))]` over your type: {}", ident,
                }
            }
        }
    }
 }

-struct Param<'a>(&'a Field, Option<&'a String>);
+#[cfg_attr(feature = "debug", derive(Debug))]
+pub struct FieldParamContainerAttributes<'a> {


It is pretty good idea to have possibility to define the style in struct level for all parameter fields, but after all since it is parameter specific attribute. Is it still possible to override this struct level style with style in parameter level with param(style = ...)?

https://github.com/juhaku/utoipa/pull/150/files#diff-bf76ef193c3d97fa4bbda36c667b8f030fe35fbbf0690dc8442bf231e7487043R284-R292 here the style container attribute will be overridden by the field attribute

Line 284-292 in into_params.rs this file

Co-authored-by: Juha Kukkonen <juha7kukkonen@gmail.com>

removed.

kellpossible · 2022-06-17T05:46:32Z

The in is indeed reserved keyword in Rust thus does not play well with naming things directly same. But not sure at the moment whether it could be used like in = Path within the macro syntax. Maybe it could be tried.

@juhaku I did try that and I think it works, but it involves having a different name for the parameter in utoipa-gen or using r#in, felt a little hacky but I can try to do it if you think it's a good idea?

juhaku · 2022-06-17T06:12:29Z

The in is indeed reserved keyword in Rust thus does not play well with naming things directly same. But not sure at the moment whether it could be used like in = Path within the macro syntax. Maybe it could be tried.

@juhaku I did try that and I think it works, but it involves having a different name for the parameter in utoipa-gen or using r#in, felt a little hacky but I can try to do it if you think it's a good idea?

I think it can be parameter_in at least it is named same as the type is its clear what represents though. 🤔

parsing logic, this is a breaking change to use Pascal case for these: `Path`, `Query`, `Header`, `Cookie`

juhaku

Awesome 🏆

kellpossible force-pushed the path-params-into-params branch from 977e94f to 39943cf Compare June 15, 2022 23:49

kellpossible added 13 commits June 16, 2022 10:00

Support accepting params(Type) for path where Type implements

0a8d742

`IntoParams`, implementing juhaku#129

Add a unit test for IntoParams used with the path macro's params()

dd6c9a6

field

Reduce scope of unit test

e7d8fa6

Implement juhaku#151 style container attribute for IntoParams

a5a4401

Add parameter_in to the IntoParams container attribute

314dd04

Updated documentation for IntoParams derive macro

0362265

Updated param() macro for IntoParams derive with ParameterIn parsing

e7fe5bc

as PascalCase instead of lowercase, to match the style parsing.

Updated path and IntoParams documentation

d0b4755

Allow both struct ident and list of parameters to be defined (fixes

251634b

actix test failure)

Updated documentation for Params

42902cd

Fixes after rebase from upstream/master

8d30f12

Fix failing doctests

1738cee

Remove duplicate Cargo.toml key asser-json-diff

df5f5ae

kellpossible force-pushed the path-params-into-params branch from 62fed6e to df5f5ae Compare June 16, 2022 00:01

kellpossible added 7 commits June 16, 2022 10:15

Remove todos and improve error handling

975c8c9

Fix bug with names not being parsed from container attribute

6f989e0

Allow params to be specified using both struct and bracket style

3a34e38

Updated path macro documentation

13c7912

Make the struct param use ExprPath instead of Ident to support paths to

b0b4e42

structs

Fix failing tests for utoipa-gen

9033a64

Fix compile error for todo-warp project

cc099bf

kellpossible mentioned this pull request Jun 16, 2022

Into params inline kellpossible/utoipa#1

Open

1 task

juhaku requested changes Jun 17, 2022

View reviewed changes

Fix typo in docs in utoipa-gen/src/lib.rs

9cb66d9

Co-authored-by: Juha Kukkonen <juha7kukkonen@gmail.com>

kellpossible and others added 4 commits June 17, 2022 15:11

Fix typo in docs in utoipa-gen/src/lib.rs

0635e9f

Co-authored-by: Juha Kukkonen <juha7kukkonen@gmail.com>

Fix typo in docs in utoipa-gen/src/lib.rs

e1e6b40

Co-authored-by: Juha Kukkonen <juha7kukkonen@gmail.com>

Remove unused Result from path.rs

8b9b9c1

Update documentation for IntoParams now that ParameterIn trait has been

4f9b1ab

removed.

Clean up attribute parsing logic to use syn::Punctuated

c85b06b

kellpossible added 2 commits June 17, 2022 20:23

Refactor ParameterIn for manual parameter definitions to use the same

451ca01

parsing logic, this is a breaking change to use Pascal case for these: `Path`, `Query`, `Header`, `Cookie`

Updated documentation for IntoParams derive

ce2359c

juhaku approved these changes Jun 18, 2022

View reviewed changes

juhaku merged commit 4b8435b into juhaku:master Jun 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Parameters as struct implementing IntoParams for warp (and other frameworks) #150

Parameters as struct implementing IntoParams for warp (and other frameworks) #150

kellpossible commented May 30, 2022 •

edited

Loading

kellpossible commented May 31, 2022

kellpossible commented May 31, 2022 •

edited

Loading

kellpossible commented May 31, 2022

kellpossible commented Jun 16, 2022

juhaku commented Jun 17, 2022

juhaku left a comment

juhaku Jun 16, 2022

juhaku Jun 16, 2022

kellpossible Jun 17, 2022

juhaku Jun 17, 2022

juhaku Jun 16, 2022

kellpossible Jun 17, 2022

kellpossible Jun 17, 2022 •

edited

Loading

juhaku Jun 17, 2022

kellpossible Jun 17, 2022

kellpossible Jun 17, 2022

juhaku Jun 18, 2022

juhaku Jun 17, 2022

kellpossible Jun 17, 2022

kellpossible Jun 17, 2022

juhaku Jun 18, 2022

kellpossible commented Jun 17, 2022

juhaku commented Jun 17, 2022

juhaku left a comment

Parameters as struct implementing IntoParams for warp (and other frameworks) #150

Parameters as struct implementing IntoParams for warp (and other frameworks) #150

Conversation

kellpossible commented May 30, 2022 • edited Loading

kellpossible commented May 31, 2022

kellpossible commented May 31, 2022 • edited Loading

kellpossible commented May 31, 2022

kellpossible commented Jun 16, 2022

juhaku commented Jun 17, 2022

juhaku left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellpossible Jun 17, 2022 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kellpossible commented Jun 17, 2022

juhaku commented Jun 17, 2022

juhaku left a comment

Choose a reason for hiding this comment

kellpossible commented May 30, 2022 •

edited

Loading

kellpossible commented May 31, 2022 •

edited

Loading

kellpossible Jun 17, 2022 •

edited

Loading