diff --git a/packages/rs-dpp/src/validation/validation_result.rs b/packages/rs-dpp/src/validation/validation_result.rs index a711e5c62f5..bc9e7bce34f 100644 --- a/packages/rs-dpp/src/validation/validation_result.rs +++ b/packages/rs-dpp/src/validation/validation_result.rs @@ -202,6 +202,10 @@ impl ValidationResult { self.errors.is_empty() } + pub fn is_err(&self) -> bool { + !self.errors.is_empty() + } + pub fn first_error(&self) -> Option<&E> { self.errors.first() } @@ -237,6 +241,19 @@ impl ValidationResult { ))) } + pub fn into_data_with_error(mut self) -> Result, ProtocolError> { + if let Some(error) = self.errors.pop() { + Ok(Err(error)) + } else { + self.data + .map(Ok) + .ok_or(ProtocolError::CorruptedCodeExecution(format!( + "trying to push validation result into data (errors are {:?})", + self.errors + ))) + } + } + pub fn into_data_and_errors(self) -> Result<(TData, Vec), ProtocolError> { Ok(( self.data diff --git a/packages/rs-drive/src/query/conditions.rs b/packages/rs-drive/src/query/conditions.rs index 1cbbd2d0169..1061501bdd3 100644 --- a/packages/rs-drive/src/query/conditions.rs +++ b/packages/rs-drive/src/query/conditions.rs @@ -3,6 +3,7 @@ use crate::error::query::QuerySyntaxError; use crate::error::Error; +use crate::query::{QuerySyntaxSimpleValidationResult, QuerySyntaxValidationResult}; #[cfg(any(feature = "server", feature = "verify"))] use dpp::data_contract::document_type::accessors::DocumentTypeV0Getters; use dpp::data_contract::document_type::methods::DocumentTypeV0Methods; @@ -164,76 +165,70 @@ impl WhereOperator { /// Matches the where operator as a SQL operator and returns it as a proper `WhereOperator` pub(crate) fn from_sql_operator(sql_operator: ast::BinaryOperator) -> Option { match sql_operator { - ast::BinaryOperator::Eq => Some(WhereOperator::Equal), - ast::BinaryOperator::Gt => Some(WhereOperator::GreaterThan), - ast::BinaryOperator::GtEq => Some(WhereOperator::GreaterThanOrEquals), - ast::BinaryOperator::Lt => Some(WhereOperator::LessThan), - ast::BinaryOperator::LtEq => Some(WhereOperator::LessThanOrEquals), + ast::BinaryOperator::Eq => Some(Equal), + ast::BinaryOperator::Gt => Some(GreaterThan), + ast::BinaryOperator::GtEq => Some(GreaterThanOrEquals), + ast::BinaryOperator::Lt => Some(LessThan), + ast::BinaryOperator::LtEq => Some(LessThanOrEquals), _ => None, } } /// Shared operator evaluator for both WhereClause and ValueClause - pub fn eval(&self, probe: &Value, clause_value: &Value) -> bool { + pub fn eval(&self, left_value: &Value, right_value: &Value) -> bool { match self { - WhereOperator::Equal => probe == clause_value, - WhereOperator::GreaterThan => probe > clause_value, - WhereOperator::GreaterThanOrEquals => probe >= clause_value, - WhereOperator::LessThan => probe < clause_value, - WhereOperator::LessThanOrEquals => probe <= clause_value, - WhereOperator::In => match clause_value { - Value::Array(array) => array.contains(probe), - Value::Bytes(bytes) => match probe { + Equal => left_value == right_value, + GreaterThan => left_value > right_value, + GreaterThanOrEquals => left_value >= right_value, + LessThan => left_value < right_value, + LessThanOrEquals => left_value <= right_value, + In => match right_value { + Value::Array(array) => array.contains(left_value), + Value::Bytes(bytes) => match left_value { Value::U8(b) => bytes.contains(b), _ => false, }, _ => false, }, - WhereOperator::Between => match clause_value { + Between => match right_value { Value::Array(bounds) if bounds.len() == 2 => { match bounds[0].partial_cmp(&bounds[1]) { - Some(Ordering::Less) | Some(Ordering::Equal) => { - probe >= &bounds[0] && probe <= &bounds[1] + Some(Ordering::Less) => { + left_value >= &bounds[0] && left_value <= &bounds[1] } _ => false, } } _ => false, }, - WhereOperator::BetweenExcludeBounds => match clause_value { + BetweenExcludeBounds => match right_value { Value::Array(bounds) if bounds.len() == 2 => { match bounds[0].partial_cmp(&bounds[1]) { - Some(Ordering::Less) | Some(Ordering::Equal) => { - probe > &bounds[0] && probe < &bounds[1] - } + Some(Ordering::Less) => left_value > &bounds[0] && left_value < &bounds[1], _ => false, } } _ => false, }, - WhereOperator::BetweenExcludeLeft => match clause_value { + BetweenExcludeLeft => match right_value { Value::Array(bounds) if bounds.len() == 2 => { match bounds[0].partial_cmp(&bounds[1]) { - Some(Ordering::Less) | Some(Ordering::Equal) => { - probe > &bounds[0] && probe <= &bounds[1] - } + Some(Ordering::Less) => left_value > &bounds[0] && left_value <= &bounds[1], _ => false, } } _ => false, }, - WhereOperator::BetweenExcludeRight => match clause_value { + BetweenExcludeRight => match right_value { Value::Array(bounds) if bounds.len() == 2 => { match bounds[0].partial_cmp(&bounds[1]) { - Some(Ordering::Less) | Some(Ordering::Equal) => { - probe >= &bounds[0] && probe < &bounds[1] - } + Some(Ordering::Less) => left_value >= &bounds[0] && left_value < &bounds[1], _ => false, } } _ => false, }, - WhereOperator::StartsWith => match (probe, clause_value) { + StartsWith => match (left_value, right_value) { (Value::Text(text), Value::Text(prefix)) => text.starts_with(prefix.as_str()), _ => false, }, @@ -244,35 +239,31 @@ impl WhereOperator { #[cfg(any(feature = "server", feature = "verify"))] pub fn value_shape_ok(&self, value: &Value, property_type: &DocumentPropertyType) -> bool { match self { - WhereOperator::Equal => true, - WhereOperator::In => matches!(value, Value::Array(_) | Value::Bytes(_)), - WhereOperator::StartsWith => matches!(value, Value::Text(_)), - WhereOperator::GreaterThan - | WhereOperator::GreaterThanOrEquals - | WhereOperator::LessThan - | WhereOperator::LessThanOrEquals => match property_type { - DocumentPropertyType::F64 => is_numeric_value(value), - DocumentPropertyType::String(_) => { - matches!(value, Value::Text(_)) + Equal => true, + In => matches!(value, Value::Array(_) | Value::Bytes(_)), + StartsWith => matches!(value, Value::Text(_)), + GreaterThan | GreaterThanOrEquals | LessThan | LessThanOrEquals => { + match property_type { + DocumentPropertyType::F64 => is_numeric_value(value), + DocumentPropertyType::String(_) => { + matches!(value, Value::Text(_)) + } + _ => matches!( + value, + Value::U128(_) + | Value::I128(_) + | Value::U64(_) + | Value::I64(_) + | Value::U32(_) + | Value::I32(_) + | Value::U16(_) + | Value::I16(_) + | Value::U8(_) + | Value::I8(_) + ), } - _ => matches!( - value, - Value::U128(_) - | Value::I128(_) - | Value::U64(_) - | Value::I64(_) - | Value::U32(_) - | Value::I32(_) - | Value::U16(_) - | Value::I16(_) - | Value::U8(_) - | Value::I8(_) - ), - }, - WhereOperator::Between - | WhereOperator::BetweenExcludeBounds - | WhereOperator::BetweenExcludeLeft - | WhereOperator::BetweenExcludeRight => { + } + Between | BetweenExcludeBounds | BetweenExcludeLeft | BetweenExcludeRight => { if let Value::Array(arr) = value { arr.len() == 2 && arr.iter().all(|x| match property_type { @@ -305,17 +296,17 @@ impl WhereOperator { impl Display for WhereOperator { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { let s = match self { - Self::Equal => "=", - Self::GreaterThan => ">", - Self::GreaterThanOrEquals => ">=", - Self::LessThan => "<", - Self::LessThanOrEquals => "<=", - Self::Between => "Between", - Self::BetweenExcludeBounds => "BetweenExcludeBounds", - Self::BetweenExcludeLeft => "BetweenExcludeLeft", - Self::BetweenExcludeRight => "BetweenExcludeRight", - Self::In => "In", - Self::StartsWith => "StartsWith", + Equal => "=", + GreaterThan => ">", + GreaterThanOrEquals => ">=", + LessThan => "<", + LessThanOrEquals => "<=", + Between => "Between", + BetweenExcludeBounds => "BetweenExcludeBounds", + BetweenExcludeLeft => "BetweenExcludeLeft", + BetweenExcludeRight => "BetweenExcludeRight", + In => "In", + StartsWith => "StartsWith", }; write!(f, "{}", s) @@ -347,37 +338,39 @@ impl<'a> WhereClause { } /// Returns the where clause `in` values if they are an array of values, else an error - pub fn in_values(&self) -> Result>, Error> { + pub fn in_values(&self) -> QuerySyntaxValidationResult>> { let in_values = match &self.value { - Value::Array(array) => Ok(Cow::Borrowed(array)), - Value::Bytes(bytes) => Ok(Cow::Owned( - bytes.iter().map(|int| Value::U8(*int)).collect(), - )), - _ => Err(Error::Query(QuerySyntaxError::InvalidInClause( - "when using in operator you must provide an array of values".to_string(), - ))), - }?; + Value::Array(array) => Cow::Borrowed(array), + Value::Bytes(bytes) => Cow::Owned(bytes.iter().map(|int| Value::U8(*int)).collect()), + _ => { + return QuerySyntaxValidationResult::new_with_error( + QuerySyntaxError::InvalidInClause( + "when using in operator you must provide an array of values".to_string(), + ), + ) + } + }; let len = in_values.len(); if len == 0 { - return Err(Error::Query(QuerySyntaxError::InvalidInClause( + return QuerySyntaxValidationResult::new_with_error(QuerySyntaxError::InvalidInClause( "in clause must have at least 1 value".to_string(), - ))); + )); } if len > 100 { - return Err(Error::Query(QuerySyntaxError::InvalidInClause( + return QuerySyntaxValidationResult::new_with_error(QuerySyntaxError::InvalidInClause( "in clause must have at most 100 values".to_string(), - ))); + )); } // Throw an error if there are duplicates if (1..in_values.len()).any(|i| in_values[i..].contains(&in_values[i - 1])) { - return Err(Error::Query(QuerySyntaxError::InvalidInClause( + return QuerySyntaxValidationResult::new_with_error(QuerySyntaxError::InvalidInClause( "there should be no duplicates values for In query".to_string(), - ))); + )); } - Ok(in_values) + QuerySyntaxValidationResult::new_with_data(in_values) } /// Returns true if the less than where clause is true @@ -600,7 +593,7 @@ impl<'a> WhereClause { let in_clauses_array = where_clauses .iter() .filter_map(|where_clause| match where_clause.operator { - WhereOperator::In => match where_clause.is_identifier() { + In => match where_clause.is_identifier() { true => None, false => Some(where_clause.clone()), }, @@ -856,7 +849,7 @@ impl<'a> WhereClause { } } In => { - let in_values = self.in_values()?; + let in_values = self.in_values().into_data_with_error()??; match starts_at_key_option { None => { @@ -1244,7 +1237,7 @@ impl<'a> WhereClause { where_clauses.push(WhereClause { field: field_name, - operator: WhereOperator::In, + operator: In, value: Value::Array(in_values), }); @@ -1256,7 +1249,7 @@ impl<'a> WhereClause { pattern, escape_char: _, } => { - let where_operator = WhereOperator::StartsWith; + let where_operator = StartsWith; if *negated { return Err(Error::Query(QuerySyntaxError::Unsupported( "Negated Like not supported".to_string(), @@ -1276,7 +1269,7 @@ impl<'a> WhereClause { )) })?; - // make sure the value is of the right format i.e prefix% + // make sure the value is of the right format i.e. prefix% let inner_text = platform_value.as_text().ok_or({ Error::Query(QuerySyntaxError::InvalidStartsWithClause( "Invalid query: startsWith takes text", @@ -1394,81 +1387,82 @@ impl<'a> WhereClause { pub fn validate_against_schema( &self, document_type: DocumentTypeRef, - ) -> Result<(), crate::error::Error> { + ) -> QuerySyntaxSimpleValidationResult { // First determine the property type of self.field let property_type_cow = if let Some(meta_ty) = meta_field_property_type(&self.field) { Cow::Owned(meta_ty) } else { // Check that the field exists in the schema - let property = document_type - .flattened_properties() - .get(&self.field) - .ok_or_else(|| { - Error::Query(QuerySyntaxError::InvalidWhereClauseComponents( - "unknown field in where clause", - )) - })?; + let Some(property) = document_type.flattened_properties().get(&self.field) else { + return QuerySyntaxSimpleValidationResult::new_with_error( + QuerySyntaxError::InvalidWhereClauseComponents("unknown field in where clause"), + ); + }; Cow::Borrowed(&property.property_type) }; // Check operator is allowed for field type let property_type = property_type_cow.as_ref(); if !allowed_ops_for_type(property_type).contains(&self.operator) { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "operator not allowed for field type", ), - )); + ); } // Check starts_with value is not empty - if self.operator == WhereOperator::StartsWith { + if self.operator == StartsWith { if let Value::Text(s) = &self.value { if s.is_empty() { - return Err(Error::Query(QuerySyntaxError::StartsWithIllegalString( - "starts_with can not start with an empty string", - ))); + return QuerySyntaxSimpleValidationResult::new_with_error( + QuerySyntaxError::StartsWithIllegalString( + "starts_with can not start with an empty string", + ), + ); } } } // Check in clause values - if self.operator == WhereOperator::In { + if self.operator == In { // Ensure array value, length bounds and no duplicates - self.in_values()?; + let result = self.in_values(); + if !result.is_valid() { + return QuerySyntaxSimpleValidationResult::new_with_errors(result.errors); + } // If value provided as Bytes, only allow for U8 numeric fields if matches!(self.value, Value::Bytes(_)) && !matches!(property_type, DocumentPropertyType::U8) { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "IN Bytes only allowed for U8 fields", ), - )); + ); } } // Check value shape is correct for operator and field type if !self.operator.value_shape_ok(&self.value, property_type) { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents("invalid value shape for operator"), - )); + ); } // For Between variants, ensure bounds are in ascending order to avoid surprising matches match self.operator { - WhereOperator::Between - | WhereOperator::BetweenExcludeBounds - | WhereOperator::BetweenExcludeLeft - | WhereOperator::BetweenExcludeRight => { + Between | BetweenExcludeBounds | BetweenExcludeLeft | BetweenExcludeRight => { if let Value::Array(bounds) = &self.value { if bounds.len() == 2 { match bounds[0].partial_cmp(&bounds[1]) { - Some(Ordering::Less) | Some(Ordering::Equal) => {} + Some(Ordering::Less) => {} _ => { - return Err(Error::Query(QuerySyntaxError::InvalidBetweenClause( - "when using between operator bounds must be ascending", - ))); + return QuerySyntaxSimpleValidationResult::new_with_error( + QuerySyntaxError::InvalidBetweenClause( + "when using between operator bounds must be strictly ascending", + ), + ); } } } @@ -1512,7 +1506,7 @@ impl<'a> WhereClause { // For equality, allow some type coercion (e.g. integer types) match self.operator { - WhereOperator::Equal => { + Equal => { use DocumentPropertyType as T; let ok = match property_type { // Accept any integer-like value for integer fields (signed/unsigned), reject floats @@ -1560,28 +1554,28 @@ impl<'a> WhereClause { T::Object(_) | T::Array(_) | T::VariableTypeArray(_) => false, }; if !ok { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "invalid value type for equality", ), - )); + ); } } - WhereOperator::In => { + In => { if let Value::Array(arr) = &self.value { if !arr.iter().all(|v| value_type_matches(property_type, v)) { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "invalid value type in IN clause", ), - )); + ); } } } _ => {} } - Ok(()) + QuerySyntaxSimpleValidationResult::new() } } @@ -1624,33 +1618,33 @@ pub fn allowed_ops_for_type(property_type: &DocumentPropertyType) -> &'static [W | DocumentPropertyType::I128 | DocumentPropertyType::F64 | DocumentPropertyType::Date => &[ - WhereOperator::Equal, - WhereOperator::In, - WhereOperator::GreaterThan, - WhereOperator::GreaterThanOrEquals, - WhereOperator::LessThan, - WhereOperator::LessThanOrEquals, - WhereOperator::Between, - WhereOperator::BetweenExcludeBounds, - WhereOperator::BetweenExcludeLeft, - WhereOperator::BetweenExcludeRight, + Equal, + In, + GreaterThan, + GreaterThanOrEquals, + LessThan, + LessThanOrEquals, + Between, + BetweenExcludeBounds, + BetweenExcludeLeft, + BetweenExcludeRight, ], DocumentPropertyType::String(_) => &[ - WhereOperator::Equal, - WhereOperator::In, - WhereOperator::StartsWith, - WhereOperator::GreaterThan, - WhereOperator::GreaterThanOrEquals, - WhereOperator::LessThan, - WhereOperator::LessThanOrEquals, - WhereOperator::Between, - WhereOperator::BetweenExcludeBounds, - WhereOperator::BetweenExcludeLeft, - WhereOperator::BetweenExcludeRight, + Equal, + In, + StartsWith, + GreaterThan, + GreaterThanOrEquals, + LessThan, + LessThanOrEquals, + Between, + BetweenExcludeBounds, + BetweenExcludeLeft, + BetweenExcludeRight, ], - DocumentPropertyType::Identifier => &[WhereOperator::Equal, WhereOperator::In], - DocumentPropertyType::ByteArray(_) => &[WhereOperator::Equal, WhereOperator::In], - DocumentPropertyType::Boolean => &[WhereOperator::Equal], + DocumentPropertyType::Identifier => &[Equal, In], + DocumentPropertyType::ByteArray(_) => &[Equal, In], + DocumentPropertyType::Boolean => &[Equal], DocumentPropertyType::Object(_) | DocumentPropertyType::Array(_) | DocumentPropertyType::VariableTypeArray(_) => &[], @@ -1684,12 +1678,12 @@ fn meta_field_property_type(field: &str) -> Option { // Dates (millis since epoch) "$createdAt" | "$updatedAt" | "$transferredAt" => Some(DocumentPropertyType::Date), // Block heights and core block heights - "$createdAtBlockHeight" - | "$updatedAtBlockHeight" - | "$transferredAtBlockHeight" - | "$createdAtCoreBlockHeight" + "$createdAtBlockHeight" | "$updatedAtBlockHeight" | "$transferredAtBlockHeight" => { + Some(DocumentPropertyType::U64) + } + "$createdAtCoreBlockHeight" | "$updatedAtCoreBlockHeight" - | "$transferredAtCoreBlockHeight" => Some(DocumentPropertyType::U64), + | "$transferredAtCoreBlockHeight" => Some(DocumentPropertyType::U32), // Revision and protocol version are integers "$revision" | "$protocolVersion" => Some(DocumentPropertyType::U64), // Type name is a string @@ -1706,9 +1700,11 @@ fn meta_field_property_type(field: &str) -> Option { #[cfg(feature = "server")] #[cfg(test)] mod tests { + use crate::error::query::QuerySyntaxError; use crate::query::conditions::WhereClause; - use crate::query::conditions::WhereOperator::{ - Equal, GreaterThan, GreaterThanOrEquals, In, LessThan, LessThanOrEquals, + use crate::query::conditions::{ + Between, BetweenExcludeBounds, BetweenExcludeLeft, BetweenExcludeRight, Equal, GreaterThan, + GreaterThanOrEquals, In, LessThan, LessThanOrEquals, ValueClause, }; use crate::query::InternalClauses; use dpp::data_contract::accessors::v0::DataContractV0Getters; @@ -1854,11 +1850,10 @@ mod tests { value: Value::Identifier([1u8; 32]), }; let res = clause.validate_against_schema(doc_type); + assert!(res.is_err()); assert!(matches!( - res, - Err(crate::error::Error::Query( - crate::error::query::QuerySyntaxError::InvalidWhereClauseComponents(_) - )) + res.first_error(), + Some(QuerySyntaxError::InvalidWhereClauseComponents(_)) )); } @@ -1879,11 +1874,10 @@ mod tests { ]), }; let res = clause.validate_against_schema(doc_type); + assert!(res.is_err()); assert!(matches!( - res, - Err(crate::error::Error::Query( - crate::error::query::QuerySyntaxError::InvalidWhereClauseComponents(_) - )) + res.first_error(), + Some(QuerySyntaxError::InvalidWhereClauseComponents(_)) )); } @@ -1906,11 +1900,10 @@ mod tests { }); let res = clauses.validate_against_schema(doc_type); + assert!(res.is_err()); assert!(matches!( - res, - Err(crate::error::Error::Query( - crate::error::query::QuerySyntaxError::InvalidWhereClauseComponents(_) - )) + res.first_error(), + Some(QuerySyntaxError::InvalidWhereClauseComponents(_)) )); } @@ -1928,11 +1921,10 @@ mod tests { value: Value::Float(1.23), }; let res = clause.validate_against_schema(doc_type); + assert!(res.is_err()); assert!(matches!( - res, - Err(crate::error::Error::Query( - crate::error::query::QuerySyntaxError::InvalidWhereClauseComponents(_) - )) + res.first_error(), + Some(QuerySyntaxError::InvalidWhereClauseComponents(_)) )); } @@ -1971,7 +1963,7 @@ mod tests { ]), }; let res = clause.validate_against_schema(doc_type); - assert!(res.is_ok()); + assert!(res.is_valid()); } #[test] @@ -1984,11 +1976,66 @@ mod tests { let clause = WhereClause { field: "$createdAt".to_string(), - operator: crate::query::conditions::WhereOperator::Between, + operator: crate::query::conditions::Between, value: Value::Array(vec![Value::U64(1000), Value::U64(2000)]), }; let res = clause.validate_against_schema(doc_type); - assert!(res.is_ok()); + assert!(res.is_valid()); + } + + #[test] + fn validate_rejects_between_variants_with_equal_bounds() { + let fixture = get_data_contract_fixture(None, 0, LATEST_PLATFORM_VERSION.protocol_version); + let contract = fixture.data_contract_owned(); + let doc_type = contract + .document_type_for_name("uniqueDates") + .expect("doc type exists"); + + for operator in [ + Between, + BetweenExcludeBounds, + BetweenExcludeLeft, + BetweenExcludeRight, + ] { + let clause = WhereClause { + field: "$createdAt".to_string(), + operator, + value: Value::Array(vec![Value::U64(1000), Value::U64(1000)]), + }; + + let res = clause.validate_against_schema(doc_type); + assert!( + res.is_err(), + "{operator:?} should reject equal bounds during validation" + ); + assert!(matches!( + res.first_error(), + Some(QuerySyntaxError::InvalidBetweenClause(_)) + )); + } + } + + #[test] + fn value_clause_between_variants_do_not_match_equal_bounds() { + let equal_bounds = Value::Array(vec![Value::U64(1000), Value::U64(1000)]); + let value_to_test = Value::U64(1000); + + for operator in [ + Between, + BetweenExcludeBounds, + BetweenExcludeLeft, + BetweenExcludeRight, + ] { + let clause = ValueClause { + operator, + value: equal_bounds.clone(), + }; + + assert!( + !clause.matches_value(&value_to_test), + "{operator:?} should not match when bounds are equal" + ); + } } #[test] @@ -2022,7 +2069,7 @@ mod tests { value: Value::U64(100), }; let res = clause.validate_against_schema(doc_type); - assert!(res.is_ok()); + assert!(res.is_valid()); } #[test] @@ -2039,6 +2086,6 @@ mod tests { value: Value::Identifier([3u8; 32]), }; let res = clause.validate_against_schema(doc_type); - assert!(res.is_ok()); + assert!(res.is_valid()); } } diff --git a/packages/rs-drive/src/query/filter.rs b/packages/rs-drive/src/query/filter.rs index e250792aeaf..9ee1b988589 100644 --- a/packages/rs-drive/src/query/filter.rs +++ b/packages/rs-drive/src/query/filter.rs @@ -42,8 +42,8 @@ use dpp::state_transition::batch_transition::document_base_transition::v0::v0_me use dpp::state_transition::batch_transition::document_replace_transition::v0::v0_methods::DocumentReplaceTransitionV0Methods; use dpp::state_transition::batch_transition::batched_transition::document_transfer_transition::v0::v0_methods::DocumentTransferTransitionV0Methods; use dpp::state_transition::batch_transition::batched_transition::document_update_price_transition::v0::v0_methods::DocumentUpdatePriceTransitionV0Methods; -use crate::query::{InternalClauses, ValueClause, WhereOperator}; -use crate::error::{query::QuerySyntaxError, Error}; +use crate::query::{InternalClauses, QuerySyntaxSimpleValidationResult, ValueClause, WhereOperator}; +use crate::error::query::QuerySyntaxError; use dpp::platform_value::ValueMapHelper; /// Filter used to match document transitions for subscriptions. @@ -432,55 +432,49 @@ impl DriveDocumentQueryFilter<'_> { /// /// In addition to these validations, the subscription host should check the contract's existence. #[cfg(any(feature = "server", feature = "verify"))] - pub fn validate(&self) -> Result<(), crate::error::Error> { + pub fn validate(&self) -> QuerySyntaxSimpleValidationResult { // Ensure the document type exists - let document_type = self + let Some(document_type) = self .contract - .document_type_for_name(&self.document_type_name) - .map_err(|_| { - Error::Query(QuerySyntaxError::DocumentTypeNotFound( - "unknown document type", - )) - })?; + .document_type_optional_for_name(&self.document_type_name) + else { + return QuerySyntaxSimpleValidationResult::new_with_error( + QuerySyntaxError::DocumentTypeNotFound("unknown document type"), + ); + }; match &self.action_clauses { DocumentActionMatchClauses::Create { new_document_clauses, - } => new_document_clauses.validate_against_schema(document_type)?, + } => new_document_clauses.validate_against_schema(document_type), DocumentActionMatchClauses::Replace { original_document_clauses, new_document_clauses, } => { - if original_document_clauses.is_empty() && new_document_clauses.is_empty() { - return Err(Error::Query( - QuerySyntaxError::InvalidWhereClauseComponents( - "replace requires at least one of original/new clauses", - ), - )); - } if !original_document_clauses.is_empty() { - original_document_clauses.validate_against_schema(document_type)?; + let result = original_document_clauses.validate_against_schema(document_type); + if result.is_err() { + return result; + } } if !new_document_clauses.is_empty() { - new_document_clauses.validate_against_schema(document_type)?; + new_document_clauses.validate_against_schema(document_type) + } else { + QuerySyntaxSimpleValidationResult::new() } } DocumentActionMatchClauses::Delete { original_document_clauses, - } => original_document_clauses.validate_against_schema(document_type)?, + } => original_document_clauses.validate_against_schema(document_type), DocumentActionMatchClauses::Transfer { original_document_clauses, owner_clause, } => { - if original_document_clauses.is_empty() && owner_clause.is_none() { - return Err(Error::Query( - QuerySyntaxError::InvalidWhereClauseComponents( - "transfer requires original clauses or owner clause", - ), - )); - } if !original_document_clauses.is_empty() { - original_document_clauses.validate_against_schema(document_type)?; + let result = original_document_clauses.validate_against_schema(document_type); + if result.is_err() { + return result; + } } if let Some(owner) = owner_clause { let ok = match owner.operator { @@ -493,26 +487,26 @@ impl DriveDocumentQueryFilter<'_> { }, _ => false, }; - if !ok { - return Err(Error::Query( + if ok { + QuerySyntaxSimpleValidationResult::new() + } else { + QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents("invalid owner clause"), - )); + ) } + } else { + QuerySyntaxSimpleValidationResult::new() } } DocumentActionMatchClauses::UpdatePrice { original_document_clauses, price_clause, } => { - if original_document_clauses.is_empty() && price_clause.is_none() { - return Err(Error::Query( - QuerySyntaxError::InvalidWhereClauseComponents( - "updatePrice requires original clauses or price clause", - ), - )); - } if !original_document_clauses.is_empty() { - original_document_clauses.validate_against_schema(document_type)?; + let result = original_document_clauses.validate_against_schema(document_type); + if result.is_err() { + return result; + } } if let Some(price) = price_clause { let ok = match price.operator { @@ -520,78 +514,48 @@ impl DriveDocumentQueryFilter<'_> { | WhereOperator::GreaterThan | WhereOperator::GreaterThanOrEquals | WhereOperator::LessThan - | WhereOperator::LessThanOrEquals => matches!( - price.value, - Value::U64(_) - | Value::I64(_) - | Value::U32(_) - | Value::I32(_) - | Value::U16(_) - | Value::I16(_) - | Value::U8(_) - | Value::I8(_) - ), + | WhereOperator::LessThanOrEquals => { + price.value.is_integer_can_fit_in_64_bits() + } WhereOperator::Between | WhereOperator::BetweenExcludeBounds | WhereOperator::BetweenExcludeLeft | WhereOperator::BetweenExcludeRight => match &price.value { Value::Array(arr) => { arr.len() == 2 - && arr.iter().all(|v| { - matches!( - v, - Value::U64(_) - | Value::I64(_) - | Value::U32(_) - | Value::I32(_) - | Value::U16(_) - | Value::I16(_) - | Value::U8(_) - | Value::I8(_) - ) - }) - && arr[0] <= arr[1] + && arr.iter().all(|v| v.is_integer_can_fit_in_64_bits()) + && arr[0] < arr[1] } _ => false, }, WhereOperator::In => match &price.value { - Value::Array(arr) => arr.iter().all(|v| { - matches!( - v, - Value::U64(_) - | Value::I64(_) - | Value::U32(_) - | Value::I32(_) - | Value::U16(_) - | Value::I16(_) - | Value::U8(_) - | Value::I8(_) - ) - }), + Value::Array(arr) => { + arr.iter().all(|v| v.is_integer_can_fit_in_64_bits()) + } _ => false, }, WhereOperator::StartsWith => false, }; - if !ok { - return Err(Error::Query( + if ok { + QuerySyntaxSimpleValidationResult::new() + } else { + QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents("invalid price clause"), - )); + ) } + } else { + QuerySyntaxSimpleValidationResult::new() } } DocumentActionMatchClauses::Purchase { original_document_clauses, owner_clause, } => { - if original_document_clauses.is_empty() && owner_clause.is_none() { - return Err(Error::Query( - QuerySyntaxError::InvalidWhereClauseComponents( - "purchase requires original clauses or owner clause", - ), - )); - } if !original_document_clauses.is_empty() { - original_document_clauses.validate_against_schema(document_type)?; + let result = original_document_clauses.validate_against_schema(document_type); + if result.is_err() { + return result; + } } if let Some(owner) = owner_clause { let ok = match owner.operator { @@ -604,15 +568,18 @@ impl DriveDocumentQueryFilter<'_> { }, _ => false, }; - if !ok { - return Err(Error::Query( + if ok { + QuerySyntaxSimpleValidationResult::new() + } else { + QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents("invalid owner clause"), - )); + ) } + } else { + QuerySyntaxSimpleValidationResult::new() } } } - Ok(()) } } @@ -927,11 +894,11 @@ mod tests { } #[test] - fn test_validate_requires_at_least_one_clause_for_optional_actions() { + fn test_validate_optional_actions_allow_empty_clauses() { let fixture = get_data_contract_fixture(None, 0, LATEST_PLATFORM_VERSION.protocol_version); let contract = fixture.data_contract_owned(); - // Replace with none/none -> invalid + // Replace with none/none -> allowed let filter = DriveDocumentQueryFilter { contract: &contract, document_type_name: "niceDocument".to_string(), @@ -940,7 +907,7 @@ mod tests { new_document_clauses: InternalClauses::default(), }, }; - assert!(filter.validate().is_err()); + assert!(filter.validate().is_valid()); // Replace with final only -> valid (non-empty final clauses) let filter = DriveDocumentQueryFilter { @@ -958,9 +925,9 @@ mod tests { }, }, }; - assert!(filter.validate().is_ok()); + assert!(filter.validate().is_valid()); - // Transfer with none/none -> invalid + // Transfer with none/none -> allowed let filter = DriveDocumentQueryFilter { contract: &contract, document_type_name: "niceDocument".to_string(), @@ -969,7 +936,7 @@ mod tests { owner_clause: None, }, }; - assert!(filter.validate().is_err()); + assert!(filter.validate().is_valid()); // Transfer with owner only -> valid let filter = DriveDocumentQueryFilter { @@ -983,9 +950,9 @@ mod tests { }), }, }; - assert!(filter.validate().is_ok()); + assert!(filter.validate().is_valid()); - // UpdatePrice with none/none -> invalid + // UpdatePrice with none/none -> allowed let filter = DriveDocumentQueryFilter { contract: &contract, document_type_name: "niceDocument".to_string(), @@ -994,7 +961,7 @@ mod tests { price_clause: None, }, }; - assert!(filter.validate().is_err()); + assert!(filter.validate().is_valid()); // UpdatePrice with price only -> valid let filter = DriveDocumentQueryFilter { @@ -1008,9 +975,9 @@ mod tests { }), }, }; - assert!(filter.validate().is_ok()); + assert!(filter.validate().is_valid()); - // Purchase with none/none -> invalid + // Purchase with none/none -> allowed let filter = DriveDocumentQueryFilter { contract: &contract, document_type_name: "niceDocument".to_string(), @@ -1019,7 +986,7 @@ mod tests { owner_clause: None, }, }; - assert!(filter.validate().is_err()); + assert!(filter.validate().is_valid()); // Purchase with owner only -> valid let filter = DriveDocumentQueryFilter { @@ -1033,7 +1000,7 @@ mod tests { }), }, }; - assert!(filter.validate().is_ok()); + assert!(filter.validate().is_valid()); } #[test] @@ -1530,7 +1497,7 @@ mod tests { }; assert!( - valid_filter.validate().is_ok(), + valid_filter.validate().is_valid(), "Filter with indexed field should be valid" ); @@ -1557,7 +1524,7 @@ mod tests { }; assert!( - invalid_filter.validate().is_ok(), + invalid_filter.validate().is_valid(), "Structural validate should ignore indexes" ); // Index-aware validation removed; structural validation suffices for subscriptions. @@ -1579,7 +1546,7 @@ mod tests { }; assert!( - primary_key_filter.validate().is_ok(), + primary_key_filter.validate().is_valid(), "Filter with only primary key should be valid" ); } @@ -1675,6 +1642,30 @@ mod tests { }, }; assert!(filter.validate().is_err()); + + // Price Between variants must reject equal bounds + for operator in [ + WhereOperator::Between, + WhereOperator::BetweenExcludeBounds, + WhereOperator::BetweenExcludeLeft, + WhereOperator::BetweenExcludeRight, + ] { + let filter = DriveDocumentQueryFilter { + contract: &contract, + document_type_name: "niceDocument".to_string(), + action_clauses: DocumentActionMatchClauses::UpdatePrice { + original_document_clauses: InternalClauses::default(), + price_clause: Some(ValueClause { + operator, + value: Value::Array(vec![Value::U64(10), Value::U64(10)]), + }), + }, + }; + assert!( + filter.validate().is_err(), + "{operator:?} should reject equal price bounds" + ); + } } #[test] diff --git a/packages/rs-drive/src/query/mod.rs b/packages/rs-drive/src/query/mod.rs index 85015a2a2a8..6253e379a67 100644 --- a/packages/rs-drive/src/query/mod.rs +++ b/packages/rs-drive/src/query/mod.rs @@ -51,6 +51,7 @@ pub use grovedb::{ use dpp::document; use dpp::prelude::Identifier; +use dpp::validation::{SimpleValidationResult, ValidationResult}; #[cfg(feature = "server")] use { crate::{drive::Drive, fees::op::LowLevelDriveOperation}, @@ -102,8 +103,8 @@ pub mod vote_polls_by_document_type_query; /// It should be implemented by the caller in order to provide data /// contract required for operations like proof verification. #[cfg(any(feature = "server", feature = "verify"))] -pub type ContractLookupFn<'a> = dyn Fn(&dpp::identifier::Identifier) -> Result>, crate::error::Error> - + 'a; +pub type ContractLookupFn<'a> = + dyn Fn(&Identifier) -> Result>, Error> + 'a; /// Creates a [ContractLookupFn] function that returns provided data contract when requested. /// @@ -119,13 +120,12 @@ pub type ContractLookupFn<'a> = dyn Fn(&dpp::identifier::Identifier) -> Result( data_contract: Arc, ) -> Box> { - let func = move - |id: &dpp::identifier::Identifier| -> Result>, crate::error::Error> { - if data_contract.id().ne(id) { - return Ok(None); - } - Ok(Some(Arc::clone(&data_contract))) - }; + let func = move |id: &Identifier| -> Result>, Error> { + if data_contract.id().ne(id) { + return Ok(None); + } + Ok(Some(Arc::clone(&data_contract))) + }; Box::new(func) } @@ -154,6 +154,12 @@ pub mod filter; #[cfg(any(feature = "server", feature = "verify"))] pub mod token_status_drive_query; +/// A Query Syntax Validation Result that contains data +pub type QuerySyntaxValidationResult = ValidationResult; + +/// A Query Syntax Validation Result +pub type QuerySyntaxSimpleValidationResult = SimpleValidationResult; + #[cfg(any(feature = "server", feature = "verify"))] /// Represents a starting point for a query based on a specific document. /// @@ -305,53 +311,62 @@ impl InternalClauses { pub fn validate_against_schema( &self, document_type: DocumentTypeRef, - ) -> Result<(), crate::error::Error> { + ) -> QuerySyntaxSimpleValidationResult { // Basic composition if !self.verify() { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "invalid composition of where clauses", ), - )); + ); } // Validate in_clause against schema if let Some(in_clause) = &self.in_clause { // Forbid $id in non-primary-key clauses if in_clause.field == "$id" { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "use primary_key_* clauses for $id", ), - )); + ); + } + let result = in_clause.validate_against_schema(document_type); + if !result.is_valid() { + return result; } - in_clause.validate_against_schema(document_type)?; } // Validate range_clause against schema if let Some(range_clause) = &self.range_clause { // Forbid $id in non-primary-key clauses if range_clause.field == "$id" { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "use primary_key_* clauses for $id", ), - )); + ); + } + let result = range_clause.validate_against_schema(document_type); + if !result.is_valid() { + return result; } - range_clause.validate_against_schema(document_type)?; } // Validate equal_clauses against schema for (field, eq_clause) in &self.equal_clauses { // Forbid $id in non-primary-key clauses if field.as_str() == "$id" { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "use primary_key_* clauses for $id", ), - )); + ); + } + let result = eq_clause.validate_against_schema(document_type); + if !result.is_valid() { + return result; } - eq_clause.validate_against_schema(document_type)?; } // Validate primary key clauses typing @@ -359,41 +374,44 @@ impl InternalClauses { if pk_eq.operator != WhereOperator::Equal || !matches!(pk_eq.value, Value::Identifier(_)) { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "primary key equality must compare an identifier", ), - )); + ); } } if let Some(pk_in) = &self.primary_key_in_clause { if pk_in.operator != WhereOperator::In { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "primary key IN must use IN operator", ), - )); + ); } // enforce array shape and no duplicates/size - pk_in.in_values()?; + let result = pk_in.in_values(); + if !result.is_valid() { + return QuerySyntaxSimpleValidationResult::new_with_errors(result.errors); + } if let Value::Array(arr) = &pk_in.value { if !arr.iter().all(|v| matches!(v, Value::Identifier(_))) { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "primary key IN must contain identifiers", ), - )); + ); } } else { - return Err(Error::Query( + return QuerySyntaxSimpleValidationResult::new_with_error( QuerySyntaxError::InvalidWhereClauseComponents( "primary key IN must contain an array of identifiers", ), - )); + ); } } - Ok(()) + QuerySyntaxSimpleValidationResult::default() } } @@ -1215,7 +1233,7 @@ impl<'a> DriveDocumentQuery<'a> { }; if let Some(primary_key_in_clause) = &self.internal_clauses.primary_key_in_clause { - let in_values = primary_key_in_clause.in_values()?; + let in_values = primary_key_in_clause.in_values().into_data_with_error()??; match starts_at_key_option { None => { diff --git a/packages/rs-platform-value/src/lib.rs b/packages/rs-platform-value/src/lib.rs index c372fc64787..5900ccdeada 100644 --- a/packages/rs-platform-value/src/lib.rs +++ b/packages/rs-platform-value/src/lib.rs @@ -148,6 +148,37 @@ impl Value { ) } + /// Returns true if the `Value` is an integer that fits in 64 bits (u64/i64). + /// Returns false otherwise. + /// + /// ``` + /// # use platform_value::Value; + /// # + /// let value = Value::U128(17); + /// + /// assert!(value.is_integer_can_fit_in_64_bits()); + /// ``` + pub fn is_integer_can_fit_in_64_bits(&self) -> bool { + match self { + // Already ≤ 64-bit widths + Value::U64(_) + | Value::I64(_) + | Value::U32(_) + | Value::I32(_) + | Value::U16(_) + | Value::I16(_) + | Value::U8(_) + | Value::I8(_) => true, + + // 128-bit -> check if within 64-bit range + Value::U128(v) => *v <= u64::MAX as u128, + Value::I128(v) => (*v >= i64::MIN as i128) && (*v <= i64::MAX as i128), + + // Non-integer variants + _ => false, + } + } + /// If the `Value` is a `Integer`, returns a reference to the associated `Integer` data. /// Returns None otherwise. ///