From acb077939ea6d2da5b123a28b6f305f125781e7a Mon Sep 17 00:00:00 2001 From: Vladimir Pesterev Date: Mon, 1 Aug 2022 16:46:53 +0400 Subject: [PATCH] [feature] #2553: Add sorting to asset queries Signed-off-by: Vladimir Pesterev --- client/src/client.rs | 17 +++- client/tests/integration/asset.rs | 96 +++++++++++++++++++ core/src/smartcontracts/isi/asset.rs | 64 ++++++++++++- data_model/src/query.rs | 35 ++++++- .../parity_scale_decoder/src/generate_map.rs | 1 + 5 files changed, 204 insertions(+), 9 deletions(-) diff --git a/client/src/client.rs b/client/src/client.rs index b34dff6226f..fc88114513f 100644 --- a/client/src/client.rs +++ b/client/src/client.rs @@ -1093,10 +1093,15 @@ pub mod asset { use super::*; /// Get query to get all assets - pub const fn all() -> FindAllAssets { + pub fn all() -> FindAllAssets { FindAllAssets::new() } + /// Get query to get all assets with specified sorting parameter + pub fn all_sorted(sort_by_metadata_key: impl Into>) -> FindAllAssets { + FindAllAssets::new().with_sort(sort_by_metadata_key) + } + /// Get query to get all asset definitions pub const fn all_definitions() -> FindAllAssetsDefinitions { FindAllAssetsDefinitions::new() @@ -1114,6 +1119,14 @@ pub mod asset { FindAssetsByAccountId::new(account_id) } + /// Get query to get all assets by account id and sort parameter + pub fn by_account_id_sorted( + account_id: impl Into>, + sort_by_metadata_key: impl Into>, + ) -> FindAssetsByAccountId { + FindAssetsByAccountId::new(account_id).with_sort(sort_by_metadata_key) + } + /// Get query to get an asset by its id pub fn by_id(asset_id: impl Into::Id>>) -> FindAssetById { FindAssetById::new(asset_id) @@ -1305,7 +1318,7 @@ mod tests { for (status_code, err) in responses { let resp = Response::builder().status(status_code).body(err.encode())?; - match sut.handle(resp) { + match sut.clone().handle(resp) { Err(ClientQueryError::QueryError(actual)) => { // PartialEq isn't implemented, so asserting by encoded repr assert_eq!(actual.encode(), err.encode()); diff --git a/client/tests/integration/asset.rs b/client/tests/integration/asset.rs index 49be2e93214..83813575047 100644 --- a/client/tests/integration/asset.rs +++ b/client/tests/integration/asset.rs @@ -235,3 +235,99 @@ fn client_add_asset_with_name_length_more_than_limit_should_not_commit_transacti Ok(()) } + +#[test] +fn correct_pagination_assets_after_creating_new_one() -> Result<()> { + let (_rt, _peer, test_client) = ::new().start_with_runtime(); + + let sort_by_metadata_key = Name::from_str("sort")?; + + let account_id = AccountId::from_str("alice@wonderland")?; + + let mut assets = vec![]; + let mut instructions: Vec = vec![]; + + for i in 0..10 { + let asset_definition_id = AssetDefinitionId::from_str(&format!("xor{}#wonderland", i))?; + let asset_definition = AssetDefinition::store(asset_definition_id.clone()); + let mut asset_metadata = Metadata::new(); + asset_metadata.insert_with_limits( + sort_by_metadata_key.clone(), + Value::U128(i), + MetadataLimits::new(10, 22), + )?; + let asset = Asset::new( + AssetId::new(asset_definition_id, account_id.clone()), + AssetValue::Store(asset_metadata), + ); + + assets.push(asset.clone()); + + let create_asset_definition = RegisterBox::new(asset_definition); + let create_asset = RegisterBox::new(asset); + + instructions.push(create_asset_definition.into()); + instructions.push(create_asset.into()); + } + + test_client.submit_all_blocking(instructions)?; + + let res = test_client.request_with_pagination( + client::asset::by_account_id_sorted(account_id.clone(), sort_by_metadata_key.clone()), + Pagination::new(Some(1), Some(5)), + )?; + + assert_eq!( + res.output + .iter() + .map(|asset| asset.id().definition_id.name.clone()) + .collect::>(), + assets + .iter() + .take(5) + .map(|asset| asset.id().definition_id.name.clone()) + .collect::>() + ); + + println!(); + + let new_asset_definition_id = AssetDefinitionId::from_str("xor10#wonderland")?; + let new_asset_definition = AssetDefinition::store(new_asset_definition_id.clone()); + let mut new_asset_metadata = Metadata::new(); + new_asset_metadata.insert_with_limits( + sort_by_metadata_key.clone(), + Value::U128(10), + MetadataLimits::new(10, 22), + )?; + let new_asset = Asset::new( + AssetId::new(new_asset_definition_id, account_id.clone()), + AssetValue::Store(new_asset_metadata), + ); + + let create_asset_definition = RegisterBox::new(new_asset_definition); + let create_asset = RegisterBox::new(new_asset.clone()); + + test_client.submit_all_blocking(vec![create_asset_definition.into(), create_asset.into()])?; + + let res = test_client.request_with_pagination( + client::asset::by_account_id_sorted(account_id, sort_by_metadata_key), + Pagination::new(Some(6), None), + )?; + + let mut right = assets.into_iter().skip(5).take(5).collect::>(); + + right.push(new_asset); + + assert_eq!( + res.output + .into_iter() + .map(|asset| asset.id().definition_id.name.clone()) + .collect::>(), + right + .into_iter() + .map(|asset| asset.id().definition_id.name.clone()) + .collect::>() + ); + + Ok(()) +} diff --git a/core/src/smartcontracts/isi/asset.rs b/core/src/smartcontracts/isi/asset.rs index 5e9710628ca..bd7a3e50cd7 100644 --- a/core/src/smartcontracts/isi/asset.rs +++ b/core/src/smartcontracts/isi/asset.rs @@ -353,6 +353,8 @@ pub mod isi { /// Asset-related query implementations. pub mod query { + use core::cmp::Ordering; + use eyre::{Result, WrapErr as _}; use super::*; @@ -361,7 +363,19 @@ pub mod query { impl ValidQuery for FindAllAssets { #[metrics(+"find_all_assets")] fn execute(&self, wsv: &WorldStateView) -> Result { + let sort_by_metadata_key = match self.sort_by_metadata_key.as_ref() { + Some(name) => Some( + name.evaluate(wsv, &Context::default()) + .wrap_err("Failed to get sorting key") + .map_err(|e| Error::Evaluate(e.to_string()))?, + ), + None => None, + }; + + iroha_logger::trace!(?sort_by_metadata_key); + let mut vec = Vec::new(); + for domain in wsv.domains().iter() { for account in domain.accounts() { for asset in account.assets() { @@ -369,6 +383,11 @@ pub mod query { } } } + + if let Some(key) = sort_by_metadata_key { + vec.sort_by(|l, r| compare_by_metadata_key(&key, l, r)); + } + Ok(vec) } } @@ -451,8 +470,25 @@ pub mod query { .evaluate(wsv, &Context::default()) .wrap_err("Failed to get account id") .map_err(|e| Error::Evaluate(e.to_string()))?; - iroha_logger::trace!(%id); - wsv.account_assets(&id).map_err(Into::into) + + let sort_by_metadata_key = match self.sort_by_metadata_key.as_ref() { + Some(name) => Some( + name.evaluate(wsv, &Context::default()) + .wrap_err("Failed to get sorting key") + .map_err(|e| Error::Evaluate(e.to_string()))?, + ), + None => None, + }; + + iroha_logger::trace!(%id, ?sort_by_metadata_key); + + let mut vec = wsv.account_assets(&id).map_err(Error::from)?; + + if let Some(key) = sort_by_metadata_key { + vec.sort_by(|l, r| compare_by_metadata_key(&key, l, r)); + } + + Ok(vec) } } @@ -587,4 +623,28 @@ pub mod query { .clone()) } } + + fn compare_by_metadata_key(key: &Name, left: &Asset, right: &Asset) -> Ordering { + let left = match left.value() { + AssetValue::Store(metadata) => metadata.get(key).unwrap_or(&Value::U128(0)), + _ => &Value::U128(0), + }; + + let left = match left { + Value::U128(value) => value, + _ => &0, + }; + + let right = match right.value() { + AssetValue::Store(metadata) => metadata.get(key).unwrap_or(&Value::U128(0)), + _ => &Value::U128(0), + }; + + let right = match right { + Value::U128(value) => value, + _ => &0, + }; + + left.cmp(right) + } } diff --git a/data_model/src/query.rs b/data_model/src/query.rs index 1621b77131c..8bd6cae90df 100644 --- a/data_model/src/query.rs +++ b/data_model/src/query.rs @@ -670,7 +670,6 @@ pub mod asset { Debug, Display, Clone, - Copy, Default, PartialEq, Eq, @@ -683,7 +682,10 @@ pub mod asset { Ord, )] #[display(fmt = "Find all assets")] - pub struct FindAllAssets; + pub struct FindAllAssets { + /// [`Name`] of the key in [`Asset`]'s metadata used to order query result. + pub sort_by_metadata_key: Option>, + } impl Query for FindAllAssets { type Output = Vec; @@ -810,6 +812,8 @@ pub mod asset { pub struct FindAssetsByAccountId { /// [`AccountId`] under which assets should be found. pub account_id: EvaluatesTo, + /// [`Name`] of the key in [`Asset`]'s metadata used to order query result. + pub sort_by_metadata_key: Option>, } impl Query for FindAssetsByAccountId { @@ -989,8 +993,18 @@ pub mod asset { impl FindAllAssets { /// Construct [`FindAllAssets`]. - pub const fn new() -> Self { - FindAllAssets + pub fn new() -> Self { + Self { + sort_by_metadata_key: None, + } + } + + /// Construct [`FindAllAssets`] with sort by key. + #[must_use] + pub fn with_sort(mut self, sort_by_metadata_key: impl Into>) -> Self { + self.sort_by_metadata_key + .replace(sort_by_metadata_key.into()); + self } } @@ -1029,7 +1043,18 @@ pub mod asset { /// Construct [`FindAssetsByAccountId`]. pub fn new(account_id: impl Into>) -> Self { let account_id = account_id.into(); - FindAssetsByAccountId { account_id } + FindAssetsByAccountId { + account_id, + sort_by_metadata_key: None, + } + } + + /// Construct [`FindAssetsByAccountId`] with sort by key. + #[must_use] + pub fn with_sort(mut self, sort_by_metadata_key: impl Into>) -> Self { + self.sort_by_metadata_key + .replace(sort_by_metadata_key.into()); + self } } diff --git a/tools/parity_scale_decoder/src/generate_map.rs b/tools/parity_scale_decoder/src/generate_map.rs index 8909f85d4d9..acd26706c99 100644 --- a/tools/parity_scale_decoder/src/generate_map.rs +++ b/tools/parity_scale_decoder/src/generate_map.rs @@ -214,6 +214,7 @@ pub fn generate_map() -> DumpDecodedMap { Option, Option, Option, + Option>, Option, Option, Option,