Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for auto-populating field default values #533

Merged
merged 4 commits into from
Mar 22, 2023

Conversation

cwatson-blackrock
Copy link
Contributor

@cwatson-blackrock cwatson-blackrock commented Mar 20, 2023

Closes #519

Thanks again for your feedback on the issue! Here is an attempt at solving it.

Summary

A named struct with #[schema(default="")] or #[serde(default)] attribute will use <struct_name>::default().<field_name> as the default value for all of its fields.
Any existing #[default] attribute on a field takes precedence.

Caveats / to do

  • I couldn't figure out how to allow #[schema(default)], as the attribute parser expects an equals sign and a value. Any ideas would be welcome!
  • I added a variant to AnyValue that represents this way of setting a default value -- I am not sure if this is very clean / consistent with how AnyValue is used currently. Please let me know if there is a better approach (same goes for naming and organization of course).

A named struct with `#[schema(default="")]` or `#[serde(default)]`
attribute will use `<struct_name>::default().<field_name>` as the
default value for all of its fields.
Any existing `#[default]` attribute on a field takes precedence.
Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

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

This looks pretty okay, 🏆

I couldn't figure out how to allow #[schema(default)], as the attribute parser expects an equals sign and a value. Any ideas would be welcome!

To be able to parse the #[schema(default)] without equals token we need to change how the parsing is being done for the Default feature. Instead of directly parsing the next token like here: https://github.com/juhaku/utoipa/pull/533/files#diff-ae9861d2ee34546b6f79f8278e17358b5345b19bf73da445e56eab12180068c4L421 it could be changed like this:

if input.peek(Token![=]) {
  parse_utils::parse_next(input, || AnyValue::parse_any(input)).map(|v| Some(Self(v)))
} else {
  Self(None)
}

And we could change the Default to accept None like struct Default(Option<AnyValue>); Or we could change any value to accept None.

@juhaku
Copy link
Owner

juhaku commented Mar 20, 2023

I added a variant to AnyValue that represents this way of setting a default value -- I am not sure if this is very clean / consistent with how AnyValue is used currently. Please let me know if there is a better approach (same goes for naming and organization of course).

I think that is okay, AnyValue is used in default and example fields which can contain arbitrary content.

- Pass `SerdeContainer` to `NamedStructSchema::field_as_schema_property`
- Syntax improvements to `field_as_schema_property`
- Support `#[schema(default)]` attribute with no parameters
@cwatson-blackrock
Copy link
Contributor Author

Excellent suggestions, thank you. I've cleaned things up and it seems to work as intended now.
Also I forgot to ask before: can / should this also be implemented for tuple structs? I'm not sure if this would play well with the representation as arrays in the schema.

Comment on lines 295 to 297
let default_feature = Feature::Default(
crate::component::features::Default::new_default_trait(struct_ident, field_ident),
);
Copy link
Owner

Choose a reason for hiding this comment

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

Also one thing came to my mind as well. If we make this default_feature a closure then we can get rid of the annoying clone() below.

let default_feature = || Feature::Default(...)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That actually also causes a move ("closure cannot be invoked more than once because it moves the variable struct_ident out of its environment"). But to avoid the clone I just realized I can also create an empty vector since the feature would get added right after:

let features_inner = field_features.get_or_insert(vec![]);
if !features_inner
    .iter()
    .any(|f| matches!(f, Feature::Default(_)))
{
     features_inner.push(default_feature);
}

I also cleaned things up a bit since default_feature no longer needs to be used twice.

@juhaku
Copy link
Owner

juhaku commented Mar 20, 2023

can / should this also be implemented for tuple structs? I'm not sure if this would play well with the representation as arrays in the schema.

I belive it is a good idea to implement them as well. Yeah with arrays it might not work very well, but with new type pattern it should be doable. The new type pattern would create schema for the inner type.

struct MyWrapper(Foobar);

impl Default for MyWrapper {
 ...
}

I think we can only are this scenario here within UnnamedFieldStruct

if all_fields_are_same {
let mut unnamed_struct_features = self.features.clone();
let value_type = unnamed_struct_features
.as_mut()
.and_then(|features| features.pop_value_type_feature());
let override_type_tree = value_type
.as_ref()
.map(|value_type| value_type.as_type_tree());
if override_type_tree.is_some() {
is_object = override_type_tree
.as_ref()
.map(|override_type| matches!(override_type.value_type, ValueType::Object))
.unwrap_or_default();
}
tokens.extend(
ComponentSchema::new(super::ComponentSchemaProps {
type_tree: override_type_tree.as_ref().unwrap_or(first_part),
features: unnamed_struct_features,
description: None,
deprecated: deprecated.as_ref(),
object_name: self.struct_name.as_ref(),
})
.to_token_stream(),
);

@cwatson-blackrock
Copy link
Contributor Author

can / should this also be implemented for tuple structs? I'm not sure if this would play well with the representation as arrays in the schema.

I belive it is a good idea to implement them as well. Yeah with arrays it might not work very well, but with new type pattern it should be doable. The new type pattern would create schema for the inner type.

Thanks. I implemented the same behavior as for named structs, for the case of a tuple struct with one field. Hopefully that covers all use cases of this feature

Copy link
Owner

@juhaku juhaku left a comment

Choose a reason for hiding this comment

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

Super 🥇 Let's get the ball rolling 🙂

@juhaku
Copy link
Owner

juhaku commented Mar 21, 2023

Thanks. I implemented the same behavior as for named structs, for the case of a tuple struct with one field. Hopefully that covers all use cases of this feature

I belive that will be good for now, until someone wants more usecases supported 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Released
Development

Successfully merging this pull request may close these issues.

Auto-populate default values in ToSchema derive macro
2 participants