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

Fix generics + nestedness in QueryMsg #1520

Merged
merged 6 commits into from
Nov 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,15 @@ and this project adheres to
[#1406]: https://github.com/CosmWasm/cosmwasm/pull/1406
[#1508]: https://github.com/CosmWasm/cosmwasm/issues/1508

### Fixed

- cosmwasm-schema: Nested QueryMsg with generics is now supported by the
QueryResponses macro ([#1516]).
- cosmwasm-schema: A nested QueryMsg no longer causes runtime errors if it
contains doc comments.

[#1516]: https://github.com/CosmWasm/cosmwasm/issues/1516

## [1.1.8] - 2022-11-22

### Fixed
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 13 additions & 6 deletions packages/schema-derive/src/query_responses.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
mod context;

use syn::{parse_quote, Expr, ExprTuple, Generics, ItemEnum, ItemImpl, Type, Variant};
use syn::{
parse_quote, Expr, ExprTuple, Generics, ItemEnum, ItemImpl, Type, TypeParamBound, Variant,
};

use self::context::Context;

Expand All @@ -13,7 +15,11 @@ pub fn query_responses_derive_impl(input: ItemEnum) -> ItemImpl {

// Handle generics if the type has any
let (_, type_generics, where_clause) = input.generics.split_for_impl();
let impl_generics = impl_generics(&ctx, &input.generics);
let impl_generics = impl_generics(
&ctx,
&input.generics,
&[parse_quote! {::cosmwasm_schema::QueryResponses}],
);

let subquery_len = subquery_calls.len();
parse_quote! {
Expand All @@ -24,7 +30,7 @@ pub fn query_responses_derive_impl(input: ItemEnum) -> ItemImpl {
let subqueries = [
#( #subquery_calls, )*
];
::cosmwasm_schema::combine_subqueries::<#subquery_len, #ident>(subqueries)
::cosmwasm_schema::combine_subqueries::<#subquery_len, #ident #type_generics>(subqueries)
}
}
}
Expand All @@ -37,7 +43,7 @@ pub fn query_responses_derive_impl(input: ItemEnum) -> ItemImpl {

// Handle generics if the type has any
let (_, type_generics, where_clause) = input.generics.split_for_impl();
let impl_generics = impl_generics(&ctx, &input.generics);
let impl_generics = impl_generics(&ctx, &input.generics, &[]);

parse_quote! {
#[automatically_derived]
Expand All @@ -55,7 +61,7 @@ pub fn query_responses_derive_impl(input: ItemEnum) -> ItemImpl {

/// Takes a list of generics from the type definition and produces a list of generics
/// for the expanded `impl` block, adding trait bounds like `JsonSchema` as appropriate.
fn impl_generics(ctx: &Context, generics: &Generics) -> Generics {
fn impl_generics(ctx: &Context, generics: &Generics, bounds: &[TypeParamBound]) -> Generics {
let mut impl_generics = generics.to_owned();
for param in impl_generics.type_params_mut() {
// remove the default type if present, as those are invalid in
Expand All @@ -65,7 +71,8 @@ fn impl_generics(ctx: &Context, generics: &Generics) -> Generics {
if !ctx.no_bounds_for.contains(&param.ident) {
param
.bounds
.push(parse_quote! {::cosmwasm_schema::schemars::JsonSchema})
.push(parse_quote! {::cosmwasm_schema::schemars::JsonSchema});
param.bounds.extend(bounds.to_owned());
}
}

Expand Down
1 change: 1 addition & 0 deletions packages/schema/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,6 @@ thiserror = "1.0.13"

[dev-dependencies]
anyhow = "1.0.57"
cosmwasm-std = { version = "1.1.8", path = "../std" }
semver = "1"
tempfile = "3"
136 changes: 1 addition & 135 deletions packages/schema/src/query_response.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
use std::collections::{BTreeMap, BTreeSet};

use schemars::{
schema::{InstanceType, RootSchema, SingleOrVec, SubschemaValidation},
JsonSchema,
};
use schemars::{schema::RootSchema, JsonSchema};
use thiserror::Error;

pub use cosmwasm_schema_derive::QueryResponses;
Expand Down Expand Up @@ -69,10 +66,6 @@ pub trait QueryResponses: JsonSchema {
fn response_schemas() -> Result<BTreeMap<String, RootSchema>, IntegrityError> {
let response_schemas = Self::response_schemas_impl();

let queries: BTreeSet<_> = response_schemas.keys().cloned().collect();

check_api_integrity::<Self>(queries)?;

Ok(response_schemas)
}

Expand All @@ -95,121 +88,6 @@ pub fn combine_subqueries<const N: usize, T>(
map
}

/// Returns possible enum variants from `one_of` analysis
fn enum_variants(
subschemas: SubschemaValidation,
) -> Result<impl Iterator<Item = Result<String, IntegrityError>>, IntegrityError> {
let iter = subschemas
.one_of
.ok_or(IntegrityError::InvalidQueryMsgSchema)?
.into_iter()
.map(|s| {
let s = s.into_object();

if let Some(SingleOrVec::Single(ty)) = s.instance_type {
match *ty {
// We'll have an object if the Rust enum variant was C-like or tuple-like
InstanceType::Object => s
.object
.ok_or(IntegrityError::InvalidQueryMsgSchema)?
.required
.into_iter()
.next()
.ok_or(IntegrityError::InvalidQueryMsgSchema),
// We might have a string here if the Rust enum variant was unit-like
InstanceType::String => {
let values = s.enum_values.ok_or(IntegrityError::InvalidQueryMsgSchema)?;

if values.len() != 1 {
return Err(IntegrityError::InvalidQueryMsgSchema);
}

values[0]
.as_str()
.map(String::from)
.ok_or(IntegrityError::InvalidQueryMsgSchema)
}
_ => Err(IntegrityError::InvalidQueryMsgSchema),
}
} else {
Err(IntegrityError::InvalidQueryMsgSchema)
}
});

Ok(iter)
}

fn verify_queries(
query_msg: BTreeSet<String>,
responses: BTreeSet<String>,
) -> Result<(), IntegrityError> {
if query_msg != responses {
return Err(IntegrityError::InconsistentQueries {
query_msg,
responses,
});
}

Ok(())
}

/// `generated_queries` is expected to be a sorted slice here!
fn check_api_integrity<T: QueryResponses + ?Sized>(
generated_queries: BTreeSet<String>,
) -> Result<(), IntegrityError> {
let schema = crate::schema_for!(T);

let subschemas = if let Some(subschemas) = schema.schema.subschemas {
subschemas
} else {
// No subschemas - no resposnes are expected
return verify_queries(BTreeSet::new(), generated_queries);
};

let schema_queries = if let Some(any_of) = subschemas.any_of {
// If `any_of` exists, we assume schema is generated from untagged enum
any_of
.into_iter()
.map(|schema| dbg!(schema.into_object()))
.filter_map(|obj| {
if let Some(reference) = obj.reference {
// Subschemas can be hidden behind references - we want to map them to proper
// subschemas in such case

// Only references to definitions are supported
let reference = match reference.strip_prefix("#/definitions/") {
Some(reference) => reference,
None => {
return Some(Err(IntegrityError::ExternalReference {
reference: reference.to_owned(),
}))
}
};

let schema = match schema.definitions.get(reference) {
Some(schema) => schema.clone(),
None => return Some(Err(IntegrityError::InvalidQueryMsgSchema)),
};

Ok(schema.into_object().subschemas).transpose()
} else {
Ok(obj.subschemas).transpose()
}
})
.map(|subschema| enum_variants(*subschema?))
.collect::<Result<Vec<_>, _>>()?
.into_iter()
.flatten()
.collect::<Result<_, _>>()?
} else {
// If `any_of` is not present, there was no untagged enum on top, we expect normal enum at
// this point
enum_variants(*subschemas)?.collect::<Result<_, _>>()?
};

verify_queries(schema_queries, generated_queries)
}

#[derive(Debug, Error, PartialEq, Eq)]
pub enum IntegrityError {
#[error("the structure of the QueryMsg schema was unexpected")]
Expand Down Expand Up @@ -299,18 +177,6 @@ mod tests {
}
}

#[test]
fn bad_msg_fails() {
let err = BadMsg::response_schemas().unwrap_err();
assert_eq!(
err,
IntegrityError::InconsistentQueries {
query_msg: BTreeSet::from(["balance-for".to_string()]),
responses: BTreeSet::from(["balance_for".to_string()])
}
);
}

#[derive(Debug, JsonSchema)]
#[serde(rename_all = "snake_case")]
#[allow(dead_code)]
Expand Down
Loading