diff --git a/crates/core/src/serde_helpers.rs b/crates/core/src/serde_helpers.rs index 3a4ffb3c..29cb1fe5 100644 --- a/crates/core/src/serde_helpers.rs +++ b/crates/core/src/serde_helpers.rs @@ -75,11 +75,36 @@ where return Err(de::Error::custom(format!("{field_name} must not be empty"))); } let mut out = HashMap::with_capacity(raw.len()); - for (key, value) in raw { - validate_placeholder_key(&key, prefix, field_name, include_size_in_too_long) + // Pass 1: validate every key first, so a malformed key is reported before + // any value error (DynamoDB validates key syntax ahead of value contents). + for key in raw.keys() { + validate_placeholder_key(key, prefix, field_name, include_size_in_too_long) .map_err(de::Error::custom)?; + } + // Pass 2: per-value check and conversion. + for (key, value) in raw { check_value(&key, &value).map_err(de::Error::custom)?; - let converted = convert(value).map_err(de::Error::custom)?; + let converted = convert(value).map_err(|e| { + let msg = e.to_string(); + // Semantic value-validation errors are wrapped with the field name + // and the offending key (DynamoDB parity). Wire/type errors (wrong + // JSON shape for a datatype) pass through as-is. + let is_value_validation = msg.starts_with("One or more parameter values were invalid:") + || msg.contains("Supplied AttributeValue is empty") + || msg.contains("Supplied AttributeValue has more than one datatypes set") + || msg.contains("cannot be converted to a numeric value") + || msg.contains("significant digits in a Number") + || msg.starts_with("Number overflow") + || msg.starts_with("Number underflow") + || msg.contains("Input collection contains duplicates"); + if is_value_validation { + de::Error::custom(format!( + "{field_name} contains invalid value: {msg} for key {key}" + )) + } else { + de::Error::custom(msg) + } + })?; out.insert(key, converted); } Ok(Some(out)) @@ -264,6 +289,162 @@ mod tests { ); } + #[test] + fn values_null_non_boolean_is_validation_error() { + // {"NULL":"no"} is a validation error on real DynamoDB, not a parse + // (Serialization) error. Must be prefixed and name its key. + let msg = values_err(r#"{"values":{":b":{"NULL":"no"}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Null attribute value types must have the value of \ + true for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_null_false_is_validation_error() { + let msg = values_err(r#"{"values":{":b":{"NULL":false}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Null attribute value types must have the value of \ + true for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_null_true_accepted() { + let parsed: TestValues = + serde_json::from_str(r#"{"values":{":b":{"NULL":true}}}"#).unwrap(); + assert!(parsed.values.is_some()); + } + + #[test] + fn values_empty_set_wrapped_with_key() { + for (av, needle) in [ + (r#"{"SS":[]}"#, "An string set may not be empty"), + (r#"{"NS":[]}"#, "An number set may not be empty"), + (r#"{"BS":[]}"#, "Binary sets should not be empty"), + ] { + let msg = values_err(&format!(r#"{{"values":{{":b":{av}}}}}"#)); + let expected = format!( + "ExpressionAttributeValues contains invalid value: One or more \ + parameter values were invalid: {needle} for key :b" + ); + assert!(msg.contains(&expected), "av={av} got: {msg}"); + } + } + + #[test] + fn values_duplicate_set_wrapped_with_key() { + let ss = values_err(r#"{"values":{":b":{"SS":["a","a"]}}}"#); + assert!( + ss.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Input collection [a, a] contains duplicates. for key :b" + ), + "{ss}" + ); + // Binary duplicates carry the "of type BS" qualifier (DynamoDB parity). + let bs = values_err(r#"{"values":{":b":{"BS":["Yg==","Yg=="]}}}"#); + assert!( + bs.contains( + "ExpressionAttributeValues contains invalid value: One or more parameter \ + values were invalid: Input collection [Yg==, Yg==]of type BS contains \ + duplicates. for key :b" + ), + "{bs}" + ); + } + + #[test] + fn values_invalid_key_reported_before_value_error() { + // Map has a malformed value (:b -> unknown type) AND a malformed key (b + // without the ':' prefix). The key error must win, matching DynamoDB. + let msg = values_err(r#"{"values":{":b":{"a":""},"b":{"S":"a"}}}"#); + assert!( + msg.contains( + r#"ExpressionAttributeValues contains invalid key: Syntax error; key: "b""# + ), + "{msg}" + ); + } + + #[test] + fn values_unsupported_datatype_wrapped_with_key() { + // An unrecognized datatype tag is reported as an empty AttributeValue, + // wrapped with the field name and key. + let msg = values_err(r#"{"values":{":b":{"a":""}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: Supplied AttributeValue \ + is empty, must contain exactly one of the supported datatypes for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_multiple_datatypes_wrapped_with_key() { + let msg = values_err(r#"{"values":{":b":{"S":"a","N":"1"}}}"#); + assert!( + msg.contains( + "ExpressionAttributeValues contains invalid value: Supplied AttributeValue \ + has more than one datatypes set, must contain exactly one of the supported \ + datatypes for key :b" + ), + "{msg}" + ); + } + + #[test] + fn values_invalid_number_wrapped_with_key() { + // Empty, non-numeric, overflow, underflow, 38-digit, and NS-duplicate + // all wrap with the field name and key, matching real DynamoDB. + for (av, needle) in [ + ( + r#"{"N":""}"#, + "The parameter cannot be converted to a numeric value", + ), + ( + r#"{"N":"b"}"#, + "The parameter cannot be converted to a numeric value: b", + ), + ( + r#"{"S":"a","N":""}"#, + "The parameter cannot be converted to a numeric value", + ), + ( + r#"{"NS":["1","b"]}"#, + "The parameter cannot be converted to a numeric value: b", + ), + ( + r#"{"NS":["1","1"]}"#, + "Input collection contains duplicates", + ), + ( + r#"{"N":"1e126"}"#, + "Number overflow. Attempting to store a number with magnitude larger", + ), + ( + r#"{"N":"1e-131"}"#, + "Number underflow. Attempting to store a number with magnitude smaller", + ), + ] { + let msg = values_err(&format!(r#"{{"values":{{":b":{av}}}}}"#)); + let expected = format!("ExpressionAttributeValues contains invalid value: {needle}"); + assert!( + msg.contains(&expected) && msg.contains("for key :b"), + "av={av} got: {msg}" + ); + } + } + #[test] fn values_key_too_long_omits_size() { let key = format!(":{}", "a".repeat(255)); // 256 bytes including ':' diff --git a/crates/core/src/types/attribute_value.rs b/crates/core/src/types/attribute_value.rs index 822729e2..7dfe0165 100755 --- a/crates/core/src/types/attribute_value.rs +++ b/crates/core/src/types/attribute_value.rs @@ -76,21 +76,50 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { } fn visit_map>(self, mut map: A) -> Result { - let (key, value): (String, serde_json::Value) = map - .next_entry()? - .ok_or_else(|| { - de::Error::custom( - "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes", - ) - })?; + let mut entries: Vec<(String, serde_json::Value)> = Vec::new(); + while let Some(entry) = map.next_entry::()? { + entries.push(entry); + } + if entries.is_empty() { + return Err(de::Error::custom( + "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes", + )); + } + + // DynamoDB validates the content of a number field before it reports the + // "more than one datatype" error, so an invalid N/NS value is surfaced + // even when several type descriptors are present. + for (k, v) in &entries { + match k.as_str() { + "N" => { + if let Some(s) = v.as_str() { + crate::validation::number::validate_and_normalize_number(s) + .map_err(|e| de::Error::custom(e.message()))?; + } + } + "NS" => { + if let Some(arr) = v.as_array() { + for elem in arr { + if let Some(s) = elem.as_str() { + crate::validation::number::validate_and_normalize_number(s) + .map_err(|e| de::Error::custom(e.message()))?; + } + } + } + } + _ => {} + } + } // REQ-TYPE-001: reject if multiple keys - if map.next_key::()?.is_some() { + if entries.len() > 1 { return Err(de::Error::custom( "Supplied AttributeValue has more than one datatypes set, must contain exactly one of the supported datatypes", )); } + let (key, value) = entries.into_iter().next().expect("entries is non-empty"); + match key.as_str() { "S" => { let s = value @@ -174,11 +203,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { }) .collect::>()?; if set.len() != arr.len() { - let values: Vec<&str> = arr.iter().filter_map(|v| v.as_str()).collect(); - let repr = values.join(", "); - return Err(de::Error::custom(format!( - "One or more parameter values were invalid: Input collection [{repr}] contains duplicates." - ))); + return Err(de::Error::custom("Input collection contains duplicates")); } Ok(AttributeValue::NS(set)) } @@ -210,7 +235,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { let values: Vec<&str> = arr.iter().filter_map(|v| v.as_str()).collect(); let repr = values.join(", "); return Err(de::Error::custom(format!( - "One or more parameter values were invalid: Input collection [{repr}] contains duplicates." + "One or more parameter values were invalid: Input collection [{repr}]of type BS contains duplicates." ))); } Ok(AttributeValue::BS(set)) @@ -222,10 +247,7 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { Ok(AttributeValue::Bool(b)) } "NULL" => { - let n = value - .as_bool() - .ok_or_else(|| de::Error::custom("NULL value must be a boolean"))?; - if !n { + if value.as_bool() != Some(true) { return Err(de::Error::custom( "One or more parameter values were invalid: Null attribute value types must have the value of true", )); @@ -256,9 +278,9 @@ impl<'de> Visitor<'de> for AttributeValueVisitor { .collect::>()?; Ok(AttributeValue::M(map)) } - other => Err(de::Error::custom(format!( - "unknown AttributeValue type descriptor: {other}" - ))), + _other => Err(de::Error::custom( + "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes", + )), } } } @@ -421,32 +443,41 @@ mod tests { #[test] fn unknown_type_descriptor_rejected() { + // An unrecognized type descriptor means no supported datatype is + // present; DynamoDB reports this as an empty AttributeValue. let json = r#"{"X":"hello"}"#; let err = serde_json::from_str::(json).unwrap_err(); - assert!(err.to_string().contains("unknown")); + assert!( + err.to_string().contains( + "Supplied AttributeValue is empty, must contain exactly one of the supported datatypes" + ), + "got: {err}" + ); } #[test] fn invalid_number_accepted_at_deserialization() { - // Invalid numbers are accepted by the deserializer (stored raw) - // and rejected later by the validation layer as ValidationException. - let json = r#"{"N":"abc"}"#; - let val: AttributeValue = serde_json::from_str(json).unwrap(); - assert_eq!(val, AttributeValue::N("abc".to_owned())); + // Invalid numbers are rejected at deserialization with DynamoDB's + // number-validation messages. + let err = serde_json::from_str::(r#"{"N":"abc"}"#).unwrap_err(); + assert!( + err.to_string() + .contains("The parameter cannot be converted to a numeric value: abc"), + "got: {err}" + ); - let json = r#"{"N":"1E999"}"#; - let val: AttributeValue = serde_json::from_str(json).unwrap(); - assert_eq!(val, AttributeValue::N("1E999".to_owned())); + let err = serde_json::from_str::(r#"{"N":"1E999"}"#).unwrap_err(); + assert!(err.to_string().contains("Number overflow"), "got: {err}"); } #[test] fn invalid_number_in_ns_accepted() { - let json = r#"{"NS":["1","abc"]}"#; - let val: AttributeValue = serde_json::from_str(json).unwrap(); - match val { - AttributeValue::NS(set) => assert!(set.contains("abc")), - _ => panic!("expected NS"), - } + let err = serde_json::from_str::(r#"{"NS":["1","abc"]}"#).unwrap_err(); + assert!( + err.to_string() + .contains("The parameter cannot be converted to a numeric value: abc"), + "got: {err}" + ); } #[test] diff --git a/crates/core/src/validation/mod.rs b/crates/core/src/validation/mod.rs index c186071a..d9ce3be3 100755 --- a/crates/core/src/validation/mod.rs +++ b/crates/core/src/validation/mod.rs @@ -961,19 +961,22 @@ pub fn validate_index_key_not_empty( let Some(value) = item.get(&ks.attribute_name) else { continue; }; - let empty = matches!(value, AttributeValue::S(s) if s.is_empty()) - || matches!(value, AttributeValue::B(b) if b.is_empty()); - if empty { + let empty_kind = match value { + AttributeValue::S(s) if s.is_empty() => Some("string"), + AttributeValue::B(b) if b.is_empty() => Some("binary"), + _ => None, + }; + if let Some(kind) = empty_kind { let msg = match ctx { SecondaryIndexEmptyContext::Item => format!( "One or more parameter values are not valid. A value specified for a secondary index key is not supported. \ - The AttributeValue for a key attribute cannot contain an empty string value. IndexName: {}, IndexKey: {}", + The AttributeValue for a key attribute cannot contain an empty {kind} value. IndexName: {}, IndexKey: {}", idx.index_name, ks.attribute_name ), - SecondaryIndexEmptyContext::UpdateExpression => + SecondaryIndexEmptyContext::UpdateExpression => format!( "One or more parameter values are not valid. The update expression attempted to update a secondary index key to a value that is not supported. \ - The AttributeValue for a key attribute cannot contain an empty string value." - .to_owned(), + The AttributeValue for a key attribute cannot contain an empty {kind} value." + ), }; return Err(DynamoDbError::ValidationException(msg)); } @@ -2124,6 +2127,42 @@ mod tests { ); } + #[test] + fn index_key_empty_binary_messages_by_context() { + // An empty BINARY value on a secondary-index key must be reported as an + // "empty binary value" (matching real DynamoDB), not "empty string value". + let owned = [idx("gsi1", "lsi1sk")]; + let refs: Vec> = owned + .iter() + .map(|(n, ks)| IndexKeyRef { + index_name: n, + key_schema: ks, + }) + .collect(); + let mut item = Item::new(); + item.insert("lsi1sk".to_owned(), AttributeValue::B(Vec::new())); + + let put_err = validate_index_key_not_empty(&item, &refs, SecondaryIndexEmptyContext::Item) + .unwrap_err(); + assert_eq!( + put_err.to_string(), + "One or more parameter values are not valid. A value specified for a secondary index key is not supported. \ + The AttributeValue for a key attribute cannot contain an empty binary value. IndexName: gsi1, IndexKey: lsi1sk" + ); + + let upd_err = validate_index_key_not_empty( + &item, + &refs, + SecondaryIndexEmptyContext::UpdateExpression, + ) + .unwrap_err(); + assert_eq!( + upd_err.to_string(), + "One or more parameter values are not valid. The update expression attempted to update a secondary index key to a value that is not supported. \ + The AttributeValue for a key attribute cannot contain an empty binary value." + ); + } + #[test] fn valid_index_key_passes() { let owned = [idx("gsi1", "lsi1sk")]; diff --git a/crates/engine/src/batch_write_item.rs b/crates/engine/src/batch_write_item.rs index b6a4dd62..7c4b6fd3 100755 --- a/crates/engine/src/batch_write_item.rs +++ b/crates/engine/src/batch_write_item.rs @@ -98,6 +98,18 @@ pub async fn handle_batch_write_item( } } + // Item-size limit is validated across the whole request before any table + // existence check, so an oversized item to a missing table returns + // ValidationException (matching Amazon DynamoDB), not + // ResourceNotFoundException. Key-schema-dependent checks stay post-existence. + for reqs in input.request_items.values() { + for wr in reqs { + if let Some(put) = &wr.put_request { + validate_item_size(&put.item, ctx.limits.max_item_size_bytes)?; + } + } + } + let empty_maps = ExpressionMaps::default(); let mut all_icm: HashMap> = HashMap::new(); @@ -124,7 +136,6 @@ pub async fn handle_batch_write_item( &key_info.attribute_definitions, )?; validate_item_nesting_depth(&put.item)?; - validate_item_size(&put.item, ctx.limits.max_item_size_bytes)?; validate_attribute_name_sizes(&put.item, &ctx.limits)?; validate_key_sizes(&put.item, &key_info.key_schema, &ctx.limits)?; diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f85e4785..806429d0 100755 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -158,6 +158,10 @@ pub(crate) fn deserialize_error(e: serde_json::Error) -> DynamoDbError { || msg.contains("AttributeValue is empty") || msg.contains("AttributeValue has more than one datatypes set") || msg.contains("parameter values were invalid") + || msg.contains("cannot be converted to a numeric value") + || msg.contains("significant digits in a Number") + || msg.contains("Number overflow") + || msg.contains("Number underflow") { DynamoDbError::ValidationException(msg) } else { diff --git a/crates/engine/src/put_item.rs b/crates/engine/src/put_item.rs index cf964fcf..25c24683 100755 --- a/crates/engine/src/put_item.rs +++ b/crates/engine/src/put_item.rs @@ -65,25 +65,7 @@ pub async fn handle_put_item( ], )?; - let input: PutItemInput = serde_json::from_value(body).map_err(|e| { - let msg = e.to_string(); - if msg.contains("parameter values were invalid") - || msg.contains("may not be empty") - || msg.contains("contains duplicates") - || msg.contains("Null attribute value") - || msg.contains("validation error detected") - || msg.contains("must not be empty") - || msg.contains("Syntax error; key") - || msg.contains("AttributeValue is empty") - || msg.contains("AttributeValue has more than one datatypes set") - { - DynamoDbError::ValidationException(msg) - } else { - DynamoDbError::SerializationException(format!( - "Start of structure or map found where not expected: {e}" - )) - } - })?; + let input: PutItemInput = serde_json::from_value(body).map_err(crate::deserialize_error)?; // Reject mixing legacy and expression parameters, then EAN/EAV supplied // without a referencing expression. @@ -148,6 +130,12 @@ pub async fn handle_put_item( )?; } + // Item-size limit is validated before the existence check so a genuinely + // oversized item to a missing table returns ValidationException (matching + // Amazon DynamoDB), not ResourceNotFoundException. Key/attribute-definition + // checks stay after existence — they need the table's key schema. + extenddb_core::validation::validate_item_size(&input.item, ctx.limits.max_item_size_bytes)?; + let key_info = ctx .table_key_info(&input.table_name) .await diff --git a/tests/conftest.py b/tests/conftest.py index c2a17182..66c5397c 100755 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -90,6 +90,34 @@ def wait_for_active(client, table_name: str, timeout: float = 120.0) -> None: return time.sleep(interval) raise TimeoutError(f"Table {table_name} did not become ACTIVE within {timeout}s") + + +def wait_for_gsi_items(paginate, expected: int, timeout: float = 15.0): + """Poll a paginated GSI query/scan until it yields at least ``expected`` items. + + GSIs are eventually consistent: an item written to the base table is not + guaranteed to be visible through a secondary index immediately — ExtendDB + applies the configured ``gsi_propagation_delay_ms``, so + a read-back through a GSI right after the write can return fewer + items than expected. Tests that write then page a GSI must poll rather than read once. + + ``paginate`` is a zero-arg callable that runs the *entire* pagination and + returns the collected list; it is retried until it returns at least + ``expected`` items or the timeout elapses. The last collected list is + returned so the caller's ordering/dedup assertions run on the converged + result. On timeout the (short) result is returned so the caller's + ``assert len(...) == expected`` fails — a genuine drop still surfaces as a + failure within the bound rather than hanging. + """ + interval = _poll_interval() + deadline = time.monotonic() + timeout + items = paginate() + while len(items) < expected and time.monotonic() < deadline: + time.sleep(interval) + items = paginate() + return items + + @pytest.fixture() def create_and_cleanup_table(dynamodb_client, unique_table_name): """Create a table and ensure it's deleted after the test (REQ-TEST-005).""" diff --git a/tests/rust/src/empty_values.rs b/tests/rust/src/empty_values.rs index 27f47902..54d18675 100755 --- a/tests/rust/src/empty_values.rs +++ b/tests/rust/src/empty_values.rs @@ -7,7 +7,7 @@ use crate::test_base::*; use aws_sdk_dynamodb::types::{ AttributeDefinition, AttributeValue, BillingMode, KeySchemaElement, KeyType, Put, - ScalarAttributeType, TransactWriteItem, WriteRequest, + ScalarAttributeType, TransactWriteItem, Update, WriteRequest, }; use std::collections::HashMap; @@ -255,6 +255,61 @@ async fn transact_write_with_empty_string() { assert_eq!(resp.item().unwrap().get("val").unwrap(), &s("")); } +/// Regression: a TransactWriteItems `Update` that sets non-key attributes to an +/// empty string and empty binary must be accepted — empty values are valid on +/// non-key attributes on real DynamoDB. Guards the transactional update path +/// against faulting (previously a server-side error on an empty-value SET). +#[tokio::test] +async fn transact_update_with_empty_string_and_binary() { + let c = client(); + let name = create_simple_table(c).await; + let mut item = HashMap::new(); + item.insert("pk".into(), s("k1")); + item.insert("val".into(), s("notempty")); + c.put_item() + .table_name(&name) + .set_item(Some(item)) + .send() + .await + .unwrap(); + c.transact_write_items() + .transact_items( + TransactWriteItem::builder() + .update( + Update::builder() + .table_name(&name) + .key("pk", s("k1")) + .update_expression("SET s_attr = :s, b_attr = :b") + .expression_attribute_values(":s", s("")) + .expression_attribute_values( + ":b", + AttributeValue::B(aws_smithy_types::Blob::new(Vec::::new())), + ) + .build() + .unwrap(), + ) + .build(), + ) + .send() + .await + .unwrap(); + let resp = c + .get_item() + .table_name(&name) + .key("pk", s("k1")) + .consistent_read(true) + .send() + .await + .unwrap(); + let got = resp.item().unwrap(); + assert_eq!(got.get("s_attr").unwrap(), &s("")); + if let AttributeValue::B(blob) = got.get("b_attr").unwrap() { + assert!(blob.as_ref().is_empty()); + } else { + panic!("Expected binary type for b_attr"); + } +} + #[tokio::test] async fn nested_empty_list() { let c = client(); diff --git a/tests/rust/src/index_key_validation.rs b/tests/rust/src/index_key_validation.rs index 7a239112..f0b4e25e 100644 --- a/tests/rust/src/index_key_validation.rs +++ b/tests/rust/src/index_key_validation.rs @@ -78,3 +78,86 @@ async fn put_item_accepts_valid_index_key() { .await .expect("valid index key must be accepted"); } + +#[tokio::test] +async fn put_item_rejects_empty_binary_index_key_reports_binary() { + use aws_sdk_dynamodb::types::{ + AttributeDefinition, AttributeValue, BillingMode, GlobalSecondaryIndex, KeySchemaElement, + KeyType, Projection, ProjectionType, ScalarAttributeType, + }; + let c = client(); + let name = format!("BinGsiEmpty{}", ts()); + c.create_table() + .table_name(&name) + .key_schema( + KeySchemaElement::builder() + .attribute_name("pk") + .key_type(KeyType::Hash) + .build() + .unwrap(), + ) + .attribute_definitions( + AttributeDefinition::builder() + .attribute_name("pk") + .attribute_type(ScalarAttributeType::S) + .build() + .unwrap(), + ) + .attribute_definitions( + AttributeDefinition::builder() + .attribute_name("gb") + .attribute_type(ScalarAttributeType::B) + .build() + .unwrap(), + ) + .global_secondary_indexes( + GlobalSecondaryIndex::builder() + .index_name("gsib") + .key_schema( + KeySchemaElement::builder() + .attribute_name("gb") + .key_type(KeyType::Hash) + .build() + .unwrap(), + ) + .projection( + Projection::builder() + .projection_type(ProjectionType::All) + .build(), + ) + .build() + .unwrap(), + ) + .billing_mode(BillingMode::PayPerRequest) + .send() + .await + .unwrap(); + wait_for_active(c, &name).await; + + // Empty BINARY value on the GSI key must be reported as an "empty binary + // value" (matching real DynamoDB), not "empty string value". + let mut item: HashMap = HashMap::new(); + item.insert("pk".into(), s("a")); + item.insert( + "gb".into(), + AttributeValue::B(aws_smithy_types::Blob::new(Vec::::new())), + ); + let err = c + .put_item() + .table_name(&name) + .set_item(Some(item)) + .send() + .await + .expect_err("empty binary index key must be rejected"); + assert_eq!( + err_code(&err), + Some("ValidationException"), + "{}", + err_msg(&err) + ); + let m = err_msg(&err); + assert!( + m.contains("empty binary value"), + "expected type-correct 'empty binary value' message, got: {m}" + ); +} diff --git a/tests/test_gsi_async.py b/tests/test_gsi_async.py index 35abbc1c..4caaa9e2 100755 --- a/tests/test_gsi_async.py +++ b/tests/test_gsi_async.py @@ -235,6 +235,12 @@ def test_gsi_sync_path_with_zero_delay( Sets the system-wide delay to 0, writes an item, and asserts the GSI query returns the item immediately (single query, no polling loop). This validates the effective_delay==0 sync path. + + Note: gsi_propagation_delay_ms is read by the data-plane write path at + server startup, so setting it at runtime here only takes effect if the + server was already booted with delay=0 (as devtools/run-tests does). A + behavioral probe below confirms the server is actually in synchronous + mode and skips — rather than falsely fails — if it is not. """ table_name = gsi_table @@ -243,6 +249,32 @@ def test_gsi_sync_path_with_zero_delay( extenddb_settings_set("gsi_propagation_delay_ms", "0") try: + # Behavioral guard: throwaway write + immediate index read. If the + # running server is genuinely synchronous, this appears at once; + # otherwise the sync path is not exercisable here — skip cleanly. + probe_gsi = f"syncprobe-gsi-{uuid.uuid4().hex[:8]}" + dynamodb_client.put_item( + TableName=table_name, + Item={ + "pk": {"S": f"syncprobe-{uuid.uuid4().hex[:8]}"}, + "gsi_pk": {"S": probe_gsi}, + "gsi_sk": {"N": "1"}, + "data": {"S": "probe"}, + }, + ) + probe = dynamodb_client.query( + TableName=table_name, + IndexName="test-gsi", + KeyConditionExpression="gsi_pk = :pk", + ExpressionAttributeValues={":pk": {"S": probe_gsi}}, + ) + if probe["Count"] != 1: + pytest.skip( + "server is not running with gsi_propagation_delay_ms=0 in " + "effect (the setting is applied at startup); the synchronous " + "GSI path cannot be exercised in this configuration" + ) + pk = f"sync-{uuid.uuid4().hex[:8]}" gsi_pk = f"sync-gsi-{uuid.uuid4().hex[:8]}" diff --git a/tests/test_query_scan.py b/tests/test_query_scan.py index 4b436cfd..5b9a4dd1 100755 --- a/tests/test_query_scan.py +++ b/tests/test_query_scan.py @@ -14,7 +14,7 @@ import pytest from botocore.exceptions import ClientError -from conftest import wait_for_active, scoped_table +from conftest import wait_for_active, wait_for_gsi_items, scoped_table @pytest.fixture(scope="class") def query_table(dynamodb_client): """Create a hash+range (S,N) table with 10 items for query tests.""" @@ -1065,27 +1065,30 @@ class TestHashOnlyGSIPagination: def test_paginate_all_items_with_limit(self, dynamodb_client, gsi_hash_only_table): """Paginating through all items on a hash-only GSI returns all 10 items.""" - all_items = [] - exclusive_start_key = None - - while True: - kwargs = { - "TableName": gsi_hash_only_table, - "IndexName": "StatusGSI", - "KeyConditionExpression": "nodeStatus = :s", - "ExpressionAttributeValues": {":s": {"S": "ACTIVE"}}, - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_hash_only_table, + "IndexName": "StatusGSI", + "KeyConditionExpression": "nodeStatus = :s", + "ExpressionAttributeValues": {":s": {"S": "ACTIVE"}}, + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 10) assert len(all_items) == 10 ids = sorted(item["instanceId"]["S"] for item in all_items) expected = sorted(f"node-{i}" for i in range(1, 11)) @@ -1190,52 +1193,58 @@ class TestGSIOnHashOnlyBaseTable: def test_paginate_duplicate_gsi_sort_keys(self, dynamodb_client, gsi_on_hash_only_base_table): """All 7 items with same GSI sort key are returned through pagination.""" - all_items = [] - exclusive_start_key = None - - while True: - kwargs = { - "TableName": gsi_on_hash_only_base_table, - "IndexName": "CategoryPriorityGSI", - "KeyConditionExpression": "category = :c", - "ExpressionAttributeValues": {":c": {"S": "urgent"}}, - "Limit": 2, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_on_hash_only_base_table, + "IndexName": "CategoryPriorityGSI", + "KeyConditionExpression": "category = :c", + "ExpressionAttributeValues": {":c": {"S": "urgent"}}, + "Limit": 2, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 7) assert len(all_items) == 7 ids = sorted(item["itemId"]["S"] for item in all_items) assert ids == sorted(f"item-{i}" for i in range(1, 8)) def test_scan_pagination_on_gsi(self, dynamodb_client, gsi_on_hash_only_base_table): """Scan on GSI with Limit paginates correctly over hash-only base table.""" - all_items = [] - exclusive_start_key = None - - while True: - kwargs = { - "TableName": gsi_on_hash_only_base_table, - "IndexName": "CategoryPriorityGSI", - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_on_hash_only_base_table, + "IndexName": "CategoryPriorityGSI", + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.scan(**kwargs) - all_items.extend(resp["Items"]) + resp = dynamodb_client.scan(**kwargs) + all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 7) assert len(all_items) == 7 ids = sorted(item["itemId"]["S"] for item in all_items) assert ids == sorted(f"item-{i}" for i in range(1, 8)) @@ -1246,7 +1255,7 @@ def test_repeated_pagination_consistent(self, dynamodb_client, gsi_on_hash_only_ The first query populates the internal base_key_cache. Subsequent queries hit the cache. Any cache corruption would cause pagination to break. """ - for run in range(5): + def _collect(): all_items = [] exclusive_start_key = None while True: @@ -1264,7 +1273,13 @@ def test_repeated_pagination_consistent(self, dynamodb_client, gsi_on_hash_only_ if "LastEvaluatedKey" not in resp: break exclusive_start_key = resp["LastEvaluatedKey"] - assert len(all_items) == 7, ( + return all_items + + # First run polls for GSI convergence; subsequent runs must stay consistent. + for run in range(5): + expected = 7 + all_items = wait_for_gsi_items(_collect, expected) if run == 0 else _collect() + assert len(all_items) == expected, ( f"Run {run+1}: expected 7 items but got {len(all_items)}" ) @@ -1278,25 +1293,29 @@ def test_paginate_reverse_gsi_with_tied_sort_keys( If the tie-breaker accidentally followed ScanIndexForward, reverse pagination would skip items or loop. """ - all_items = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": gsi_on_hash_only_base_table, - "IndexName": "CategoryPriorityGSI", - "KeyConditionExpression": "category = :c", - "ExpressionAttributeValues": {":c": {"S": "urgent"}}, - "ScanIndexForward": False, - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": gsi_on_hash_only_base_table, + "IndexName": "CategoryPriorityGSI", + "KeyConditionExpression": "category = :c", + "ExpressionAttributeValues": {":c": {"S": "urgent"}}, + "ScanIndexForward": False, + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 7) assert len(all_items) == 7, ( f"Reverse GSI pagination: expected 7 items but got {len(all_items)}" ) @@ -1382,24 +1401,28 @@ def test_index_pagination_uses_base_key_schema_for_tiebreaker( self, dynamodb_client, base_key_schema_table ): """GSI query: base_key_schema differs from key_schema, used for tie-breaking.""" - all_items = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": base_key_schema_table, - "IndexName": "TestGSI", - "KeyConditionExpression": "gsi_pk = :gpk", - "ExpressionAttributeValues": {":gpk": {"S": "shared"}}, - "Limit": 3, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.query(**kwargs) - all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": base_key_schema_table, + "IndexName": "TestGSI", + "KeyConditionExpression": "gsi_pk = :gpk", + "ExpressionAttributeValues": {":gpk": {"S": "shared"}}, + "Limit": 3, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.query(**kwargs) + all_items.extend(resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 12) assert len(all_items) == 12 # No duplicates across pages item_keys = [(i["pk"]["S"], i["sk"]["N"]) for i in all_items] @@ -1409,22 +1432,26 @@ def test_index_scan_pagination_uses_base_key_schema( self, dynamodb_client, base_key_schema_table ): """GSI scan: base_key_schema provides base PK/SK for compound pagination.""" - all_items = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": base_key_schema_table, - "IndexName": "TestGSI", - "Limit": 4, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.scan(**kwargs) - all_items.extend(resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_items = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": base_key_schema_table, + "IndexName": "TestGSI", + "Limit": 4, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.scan(**kwargs) + all_items.extend(resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_items + # GSI is eventually consistent — poll until all writes have propagated. + all_items = wait_for_gsi_items(_collect, 12) assert len(all_items) == 12 item_keys = [(i["pk"]["S"], i["sk"]["N"]) for i in all_items] assert len(item_keys) == len(set(item_keys)) @@ -1477,22 +1504,26 @@ class TestHashOnlyGsiCompositeBaseScan: def test_scan_paginate_returns_all( self, dynamodb_client, hash_only_gsi_on_composite_base ): - all_ts = [] - exclusive_start_key = None - while True: - kwargs = { - "TableName": hash_only_gsi_on_composite_base, - "IndexName": "StatusGSI", - "Limit": 2, - } - if exclusive_start_key: - kwargs["ExclusiveStartKey"] = exclusive_start_key - resp = dynamodb_client.scan(**kwargs) - all_ts.extend(item["ts"]["N"] for item in resp["Items"]) - if "LastEvaluatedKey" not in resp: - break - exclusive_start_key = resp["LastEvaluatedKey"] + def _collect(): + all_ts = [] + exclusive_start_key = None + while True: + kwargs = { + "TableName": hash_only_gsi_on_composite_base, + "IndexName": "StatusGSI", + "Limit": 2, + } + if exclusive_start_key: + kwargs["ExclusiveStartKey"] = exclusive_start_key + resp = dynamodb_client.scan(**kwargs) + all_ts.extend(item["ts"]["N"] for item in resp["Items"]) + if "LastEvaluatedKey" not in resp: + break + exclusive_start_key = resp["LastEvaluatedKey"] + return all_ts + # GSI is eventually consistent — poll until all writes have propagated. + all_ts = wait_for_gsi_items(_collect, 7) assert len(all_ts) == 7, f"expected 7 items, got {len(all_ts)}" assert sorted(all_ts, key=int) == [str(i) for i in range(1, 8)] diff --git a/tests/test_validation_precedence.py b/tests/test_validation_precedence.py index 69bc8bf8..96466445 100644 --- a/tests/test_validation_precedence.py +++ b/tests/test_validation_precedence.py @@ -28,9 +28,15 @@ it against an existing table (Amazon DynamoDB requires a Value). Some other checks are intentionally *post-existence* on Amazon DynamoDB and -must keep returning ``ResourceNotFoundException`` for an absent table. The most -visible one is the item-size limit (PutItem / BatchWriteItem). The control -tests at the bottom pin that boundary so the fix does not over-correct. +must keep returning ``ResourceNotFoundException`` for an absent table (the +key-schema-dependent checks, which need the table's key schema). + +The item-size limit (PutItem / BatchWriteItem) is *pre-existence*: a genuinely +oversized item (> the 400 KB / 409,600-byte limit) to an absent table returns +``ValidationException``. An earlier revision of this file claimed the opposite, +but its "big" item was ``"x" * 400001`` (~400,004 bytes) — actually *under* the +limit, so it was never oversized and returned ``ResourceNotFoundException`` for +the wrong reason. Verified against Amazon DynamoDB with a >400 KB item. All expected behaviour here was captured from Amazon DynamoDB via the AWS CLI (profile ``asomasun-admin``, us-east-1). @@ -172,29 +178,29 @@ def test_batch_get_item_duplicate_keys(self, client): _assert_validation(ei, "duplicates") -class TestValidationAfterExistence: - """Controls: checks that stay post-existence on Amazon DynamoDB. +class TestItemSizeBeforeExistence: + """Item-size limit is validated *before* the existence check. - These must keep returning ResourceNotFoundException for an absent table so - the precedence fix does not move item-content validation ahead of the - existence check. + Verified against Amazon DynamoDB: a genuinely oversized item (> the 400 KB / + 409,600-byte limit) to an absent table returns ``ValidationException``, not + ``ResourceNotFoundException``. (A sub-limit item to an absent table correctly + returns ``ResourceNotFoundException`` because it is not oversized.) """ - def _big_item(self) -> dict: - return {"a": {"S": "k"}, "b": {"S": "x" * 400001}} + def _oversized_item(self) -> dict: + # > 400 KB (409,600 bytes): a single ~410 KB attribute value. + return {"a": {"S": "k"}, "b": {"S": "x" * 410000}} - def test_put_item_too_big_is_resource_not_found(self, client): + def test_put_item_too_big_before_existence(self, client): with pytest.raises(ClientError) as ei: - client.put_item(TableName=ABSENT_TABLE, Item=self._big_item()) - code, _ = _error(ei) - assert code == "ResourceNotFoundException", f"got {code}" + client.put_item(TableName=ABSENT_TABLE, Item=self._oversized_item()) + _assert_validation(ei, "Item size has exceeded the maximum allowed size") - def test_batch_write_item_too_big_is_resource_not_found(self, client): + def test_batch_write_item_too_big_before_existence(self, client): with pytest.raises(ClientError) as ei: client.batch_write_item( RequestItems={ - ABSENT_TABLE: [{"PutRequest": {"Item": self._big_item()}}] + ABSENT_TABLE: [{"PutRequest": {"Item": self._oversized_item()}}] }, ) - code, _ = _error(ei) - assert code == "ResourceNotFoundException", f"got {code}" + _assert_validation(ei, "Item size has exceeded the maximum allowed size")