Skip to content

Commit 1757858

Browse files
jct-tympanonjct-tympanon
committed
consolidate nullish deserializers.
addressed review feedback for #1129 . all of the implementations for the removed functions were identical, so they are exactly equivalent to the generic deserialize_nullish. existing unit tests demonstrate the equivalence. functions are private to the crate and safe to remove. I added one assertion which demonstrates failing behavior for missing fields (which are *not* covered by this deserializer). the behavior for that case is the same before and after the change, it just wasn't tested. also updated field documents and the helper function to explain the semantics.
1 parent d740e6f commit 1757858

File tree

23 files changed

+140
-170
lines changed

23 files changed

+140
-170
lines changed

‎lambda-events/src/custom_serde/mod.rs‎

Lines changed: 16 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use serde::{
33
de::{Deserialize, Deserializer, Error as DeError},
44
ser::Serializer,
55
};
6-
use std::collections::HashMap;
76

87
#[cfg(feature = "codebuild")]
98
pub(crate) mod codebuild_time;
@@ -59,6 +58,12 @@ where
5958
}
6059

6160
/// Deserializes any `Default` type, mapping JSON `null` to `T::default()`.
61+
///
62+
/// **Note** null-to-empty semantics are usually clear for container types (Map, Vec, etc).
63+
/// For most other data types, prefer modeling fields as Option<T> with #[serde(default)]
64+
/// instead of using this deserializer. Option preserves information about the message
65+
/// for the application, and default semantics for the target data type may change
66+
/// over time without warning.
6267
pub(crate) fn deserialize_nullish<'de, D, T>(deserializer: D) -> Result<T, D::Error>
6368
where
6469
D: Deserializer<'de>,
@@ -68,56 +73,13 @@ where
6873
Ok(opt.unwrap_or_default())
6974
}
7075

71-
/// Deserializes `HashMap<_>`, mapping JSON `null` to an empty map.
72-
pub(crate) fn deserialize_lambda_map<'de, D, K, V>(deserializer: D) -> Result<HashMap<K, V>, D::Error>
73-
where
74-
D: Deserializer<'de>,
75-
K: serde::Deserialize<'de>,
76-
K: std::hash::Hash,
77-
K: std::cmp::Eq,
78-
V: serde::Deserialize<'de>,
79-
{
80-
// https://github.com/serde-rs/serde/issues/1098
81-
let opt = Option::deserialize(deserializer)?;
82-
Ok(opt.unwrap_or_default())
83-
}
84-
85-
#[cfg(feature = "dynamodb")]
86-
/// Deserializes `Item`, mapping JSON `null` to an empty item.
87-
pub(crate) fn deserialize_lambda_dynamodb_item<'de, D>(deserializer: D) -> Result<serde_dynamo::Item, D::Error>
88-
where
89-
D: Deserializer<'de>,
90-
{
91-
// https://github.com/serde-rs/serde/issues/1098
92-
let opt = Option::deserialize(deserializer)?;
93-
Ok(opt.unwrap_or_default())
94-
}
95-
96-
/// Deserializes `HashMap<_>`, mapping JSON `null` to an empty map.
97-
#[cfg(any(
98-
feature = "alb",
99-
feature = "apigw",
100-
feature = "cloudwatch_events",
101-
feature = "code_commit",
102-
feature = "cognito",
103-
feature = "sns",
104-
feature = "vpc_lattice",
105-
test
106-
))]
107-
pub(crate) fn deserialize_nullish_boolean<'de, D>(deserializer: D) -> Result<bool, D::Error>
108-
where
109-
D: Deserializer<'de>,
110-
{
111-
// https://github.com/serde-rs/serde/issues/1098
112-
let opt = Option::deserialize(deserializer)?;
113-
Ok(opt.unwrap_or_default())
114-
}
115-
11676
#[cfg(test)]
11777
#[allow(deprecated)]
11878
mod test {
11979
use super::*;
80+
12081
use serde::{Deserialize, Serialize};
82+
use std::collections::HashMap;
12183

12284
#[test]
12385
fn test_deserialize_base64() {
@@ -151,7 +113,7 @@ mod test {
151113
fn test_deserialize_map() {
152114
#[derive(Deserialize)]
153115
struct Test {
154-
#[serde(deserialize_with = "deserialize_lambda_map")]
116+
#[serde(deserialize_with = "deserialize_nullish")]
155117
v: HashMap<String, String>,
156118
}
157119
let input = serde_json::json!({
@@ -170,9 +132,9 @@ mod test {
170132
#[cfg(feature = "dynamodb")]
171133
#[test]
172134
fn test_deserialize_lambda_dynamodb_item() {
173-
#[derive(Deserialize)]
135+
#[derive(Deserialize, Debug)]
174136
struct Test {
175-
#[serde(deserialize_with = "deserialize_lambda_dynamodb_item")]
137+
#[serde(deserialize_with = "deserialize_nullish")]
176138
v: serde_dynamo::Item,
177139
}
178140
let input = serde_json::json!({
@@ -186,6 +148,10 @@ mod test {
186148
});
187149
let decoded: Test = serde_json::from_value(input).unwrap();
188150
assert_eq!(serde_dynamo::Item::from(HashMap::new()), decoded.v);
151+
152+
let input = serde_json::json!({});
153+
let failure = serde_json::from_value::<Test>(input);
154+
assert!(failure.is_err(), "Missing field should not default: {failure:?}")
189155
}
190156

191157
#[test]
@@ -214,7 +180,7 @@ mod test {
214180
fn test_deserialize_nullish_boolean() {
215181
#[derive(Deserialize)]
216182
struct Test {
217-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
183+
#[serde(default, deserialize_with = "deserialize_nullish")]
218184
v: bool,
219185
}
220186

‎lambda-events/src/event/activemq/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize};
55
use serde_json::Value;
66
use std::collections::HashMap;
77

8-
use crate::custom_serde::deserialize_lambda_map;
8+
use crate::custom_serde::deserialize_nullish;
99

1010
#[non_exhaustive]
1111
#[cfg_attr(feature = "builders", derive(Builder))]
@@ -54,7 +54,7 @@ pub struct ActiveMqMessage {
5454
pub data: Option<String>,
5555
pub broker_in_time: i64,
5656
pub broker_out_time: i64,
57-
#[serde(deserialize_with = "deserialize_lambda_map")]
57+
#[serde(deserialize_with = "deserialize_nullish")]
5858
#[serde(default)]
5959
pub properties: HashMap<String, String>,
6060
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.

‎lambda-events/src/event/alb/mod.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
custom_serde::{
3-
deserialize_headers, deserialize_nullish_boolean, http_method, serialize_headers,
3+
deserialize_headers, deserialize_nullish, http_method, serialize_headers,
44
serialize_multi_value_headers, serialize_query_string_parameters,
55
},
66
encodings::Body,
@@ -35,7 +35,7 @@ pub struct AlbTargetGroupRequest {
3535
#[serde(serialize_with = "serialize_multi_value_headers")]
3636
pub multi_value_headers: HeaderMap,
3737
pub request_context: AlbTargetGroupRequestContext,
38-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
38+
#[serde(default, deserialize_with = "deserialize_nullish")]
3939
pub is_base64_encoded: bool,
4040
pub body: Option<String>,
4141
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
@@ -101,7 +101,7 @@ pub struct AlbTargetGroupResponse {
101101
pub multi_value_headers: HeaderMap,
102102
#[serde(skip_serializing_if = "Option::is_none")]
103103
pub body: Option<Body>,
104-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
104+
#[serde(default, deserialize_with = "deserialize_nullish")]
105105
pub is_base64_encoded: bool,
106106
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
107107
/// Enabled with Cargo feature `catch-all-fields`.

‎lambda-events/src/event/apigw/mod.rs‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
custom_serde::{
3-
deserialize_headers, deserialize_lambda_map, deserialize_nullish_boolean, http_method, serialize_headers,
3+
deserialize_headers, deserialize_nullish, http_method, serialize_headers,
44
serialize_multi_value_headers,
55
},
66
encodings::Body,
@@ -39,17 +39,17 @@ pub struct ApiGatewayProxyRequest {
3939
pub query_string_parameters: QueryMap,
4040
#[serde(default, deserialize_with = "query_map::serde::standard::deserialize_empty")]
4141
pub multi_value_query_string_parameters: QueryMap,
42-
#[serde(deserialize_with = "deserialize_lambda_map")]
42+
#[serde(deserialize_with = "deserialize_nullish")]
4343
#[serde(default)]
4444
pub path_parameters: HashMap<String, String>,
45-
#[serde(deserialize_with = "deserialize_lambda_map")]
45+
#[serde(deserialize_with = "deserialize_nullish")]
4646
#[serde(default)]
4747
pub stage_variables: HashMap<String, String>,
4848
#[serde(bound = "")]
4949
pub request_context: ApiGatewayProxyRequestContext,
5050
#[serde(default)]
5151
pub body: Option<String>,
52-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
52+
#[serde(default, deserialize_with = "deserialize_nullish")]
5353
pub is_base64_encoded: bool,
5454
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
5555
/// Enabled with Cargo feature `catch-all-fields`.
@@ -76,7 +76,7 @@ pub struct ApiGatewayProxyResponse {
7676
pub multi_value_headers: HeaderMap,
7777
#[serde(skip_serializing_if = "Option::is_none")]
7878
pub body: Option<Body>,
79-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
79+
#[serde(default, deserialize_with = "deserialize_nullish")]
8080
pub is_base64_encoded: bool,
8181
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
8282
/// Enabled with Cargo feature `catch-all-fields`.
@@ -238,12 +238,12 @@ pub struct ApiGatewayV2httpRequest {
238238
#[serde(skip_serializing_if = "QueryMap::is_empty")]
239239
#[serde(serialize_with = "query_map::serde::aws_api_gateway_v2::serialize_query_string_parameters")]
240240
pub query_string_parameters: QueryMap,
241-
#[serde(deserialize_with = "deserialize_lambda_map")]
241+
#[serde(deserialize_with = "deserialize_nullish")]
242242
#[serde(default)]
243243
#[serde(skip_serializing_if = "HashMap::is_empty")]
244244
pub path_parameters: HashMap<String, String>,
245245
pub request_context: ApiGatewayV2httpRequestContext,
246-
#[serde(deserialize_with = "deserialize_lambda_map")]
246+
#[serde(deserialize_with = "deserialize_nullish")]
247247
#[serde(default)]
248248
pub stage_variables: HashMap<String, String>,
249249
#[serde(skip_serializing_if = "Option::is_none")]
@@ -314,7 +314,7 @@ pub struct ApiGatewayRequestAuthorizer {
314314
rename = "lambda",
315315
default,
316316
skip_serializing_if = "HashMap::is_empty",
317-
deserialize_with = "deserialize_lambda_map"
317+
deserialize_with = "deserialize_nullish"
318318
)]
319319
pub fields: HashMap<String, Value>,
320320
#[serde(skip_serializing_if = "Option::is_none")]
@@ -335,7 +335,7 @@ pub struct ApiGatewayRequestAuthorizer {
335335
#[derive(Clone, Debug, Default, Deserialize, Eq, PartialEq, Serialize)]
336336
#[serde(rename_all = "camelCase")]
337337
pub struct ApiGatewayRequestAuthorizerJwtDescription {
338-
#[serde(deserialize_with = "deserialize_lambda_map")]
338+
#[serde(deserialize_with = "deserialize_nullish")]
339339
#[serde(default)]
340340
pub claims: HashMap<String, String>,
341341
#[serde(skip_serializing_if = "Option::is_none")]
@@ -442,7 +442,7 @@ pub struct ApiGatewayV2httpResponse {
442442
pub multi_value_headers: HeaderMap,
443443
#[serde(skip_serializing_if = "Option::is_none")]
444444
pub body: Option<Body>,
445-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
445+
#[serde(default, deserialize_with = "deserialize_nullish")]
446446
pub is_base64_encoded: bool,
447447
pub cookies: Vec<String>,
448448
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
@@ -525,17 +525,17 @@ pub struct ApiGatewayWebsocketProxyRequest {
525525
pub query_string_parameters: QueryMap,
526526
#[serde(default, deserialize_with = "query_map::serde::standard::deserialize_empty")]
527527
pub multi_value_query_string_parameters: QueryMap,
528-
#[serde(deserialize_with = "deserialize_lambda_map")]
528+
#[serde(deserialize_with = "deserialize_nullish")]
529529
#[serde(default)]
530530
pub path_parameters: HashMap<String, String>,
531-
#[serde(deserialize_with = "deserialize_lambda_map")]
531+
#[serde(deserialize_with = "deserialize_nullish")]
532532
#[serde(default)]
533533
pub stage_variables: HashMap<String, String>,
534534
#[serde(bound = "")]
535535
pub request_context: ApiGatewayWebsocketProxyRequestContext,
536536
#[serde(default)]
537537
pub body: Option<String>,
538-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
538+
#[serde(default, deserialize_with = "deserialize_nullish")]
539539
pub is_base64_encoded: bool,
540540
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
541541
/// Enabled with Cargo feature `catch-all-fields`.
@@ -815,13 +815,13 @@ pub struct ApiGatewayV2CustomAuthorizerV1Request {
815815
#[serde(deserialize_with = "http_serde::header_map::deserialize", default)]
816816
#[serde(serialize_with = "serialize_headers")]
817817
pub headers: HeaderMap,
818-
#[serde(deserialize_with = "deserialize_lambda_map")]
818+
#[serde(deserialize_with = "deserialize_nullish")]
819819
#[serde(default)]
820820
pub query_string_parameters: HashMap<String, String>,
821-
#[serde(deserialize_with = "deserialize_lambda_map")]
821+
#[serde(deserialize_with = "deserialize_nullish")]
822822
#[serde(default)]
823823
pub path_parameters: HashMap<String, String>,
824-
#[serde(deserialize_with = "deserialize_lambda_map")]
824+
#[serde(deserialize_with = "deserialize_nullish")]
825825
#[serde(default)]
826826
pub stage_variables: HashMap<String, String>,
827827
pub request_context: ApiGatewayV2CustomAuthorizerV1RequestTypeRequestContext,
@@ -860,14 +860,14 @@ pub struct ApiGatewayV2CustomAuthorizerV2Request {
860860
#[serde(deserialize_with = "http_serde::header_map::deserialize", default)]
861861
#[serde(serialize_with = "serialize_headers")]
862862
pub headers: HeaderMap,
863-
#[serde(deserialize_with = "deserialize_lambda_map")]
863+
#[serde(deserialize_with = "deserialize_nullish")]
864864
#[serde(default)]
865865
pub query_string_parameters: HashMap<String, String>,
866866
pub request_context: ApiGatewayV2httpRequestContext,
867-
#[serde(deserialize_with = "deserialize_lambda_map")]
867+
#[serde(deserialize_with = "deserialize_nullish")]
868868
#[serde(default)]
869869
pub path_parameters: HashMap<String, String>,
870-
#[serde(deserialize_with = "deserialize_lambda_map")]
870+
#[serde(deserialize_with = "deserialize_nullish")]
871871
#[serde(default)]
872872
pub stage_variables: HashMap<String, String>,
873873
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
@@ -890,7 +890,7 @@ pub struct ApiGatewayCustomAuthorizerContext {
890890
pub principal_id: Option<String>,
891891
pub string_key: Option<String>,
892892
pub num_key: Option<i64>,
893-
#[serde(default, deserialize_with = "deserialize_nullish_boolean")]
893+
#[serde(default, deserialize_with = "deserialize_nullish")]
894894
pub bool_key: bool,
895895
/// Catchall to catch any additional fields that were present but not explicitly defined by this struct.
896896
/// Enabled with Cargo feature `catch-all-fields`.
@@ -993,10 +993,10 @@ pub struct ApiGatewayCustomAuthorizerRequestTypeRequest {
993993
pub query_string_parameters: QueryMap,
994994
#[serde(default, deserialize_with = "query_map::serde::standard::deserialize_empty")]
995995
pub multi_value_query_string_parameters: QueryMap,
996-
#[serde(deserialize_with = "deserialize_lambda_map")]
996+
#[serde(deserialize_with = "deserialize_nullish")]
997997
#[serde(default)]
998998
pub path_parameters: HashMap<String, String>,
999-
#[serde(deserialize_with = "deserialize_lambda_map")]
999+
#[serde(deserialize_with = "deserialize_nullish")]
10001000
#[serde(default)]
10011001
pub stage_variables: HashMap<String, String>,
10021002
pub request_context: ApiGatewayCustomAuthorizerRequestTypeRequestContext,

‎lambda-events/src/event/appsync/mod.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize};
44
use serde_json::Value;
55
use std::collections::HashMap;
66

7-
use crate::custom_serde::deserialize_lambda_map;
7+
use crate::custom_serde::deserialize_nullish;
88

99
/// Deprecated: `AppSyncResolverTemplate` does not represent resolver events sent by AppSync. Instead directly model your input schema, or use `map[string]string`, `json.RawMessage`,` interface{}`, etc..
1010
#[non_exhaustive]
@@ -79,7 +79,7 @@ where
7979
pub issuer: Option<String>,
8080
#[serde(default)]
8181
pub username: Option<String>,
82-
#[serde(deserialize_with = "deserialize_lambda_map")]
82+
#[serde(deserialize_with = "deserialize_nullish")]
8383
#[serde(default)]
8484
#[serde(bound = "")]
8585
pub claims: HashMap<String, T1>,
@@ -139,7 +139,7 @@ where
139139
pub query_string: Option<String>,
140140
#[serde(default)]
141141
pub operation_name: Option<String>,
142-
#[serde(deserialize_with = "deserialize_lambda_map")]
142+
#[serde(deserialize_with = "deserialize_nullish")]
143143
#[serde(default)]
144144
#[serde(bound = "")]
145145
pub variables: HashMap<String, T1>,
@@ -164,7 +164,7 @@ where
164164
T1: Serialize,
165165
{
166166
pub is_authorized: bool,
167-
#[serde(deserialize_with = "deserialize_lambda_map")]
167+
#[serde(deserialize_with = "deserialize_nullish")]
168168
#[serde(default)]
169169
#[serde(bound = "")]
170170
pub resolver_context: HashMap<String, T1>,
@@ -229,7 +229,7 @@ where
229229
#[derive(Debug, Default, Clone, Eq, PartialEq, Deserialize, Serialize)]
230230
#[serde(rename_all = "camelCase")]
231231
pub struct AppSyncRequest {
232-
#[serde(deserialize_with = "deserialize_lambda_map")]
232+
#[serde(deserialize_with = "deserialize_nullish")]
233233
#[serde(default)]
234234
#[serde(bound = "")]
235235
pub headers: HashMap<String, Option<String>>,

‎lambda-events/src/event/autoscaling/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use serde::{de::DeserializeOwned, Deserialize, Serialize};
55
use serde_json::Value;
66
use std::collections::HashMap;
77

8-
use crate::custom_serde::deserialize_lambda_map;
8+
use crate::custom_serde::deserialize_nullish;
99

1010
/// `AutoScalingEvent` struct is used to parse the json for auto scaling event types //
1111
#[non_exhaustive]
@@ -41,7 +41,7 @@ where
4141
pub region: Option<String>,
4242
/// Information about resources impacted by event
4343
pub resources: Vec<String>,
44-
#[serde(deserialize_with = "deserialize_lambda_map")]
44+
#[serde(deserialize_with = "deserialize_nullish")]
4545
#[serde(default)]
4646
#[serde(bound = "")]
4747
pub detail: HashMap<String, T1>,

0 commit comments

Comments
 (0)