Skip to content

Commit 00f4602

Browse files
Huliiiiiityt2y3
Huliiiiii
and
tyt2y3
authored
Fix serde(rename_all) breaking ActiveModel::from_json field defaulting (#2855)
Fixes a regression from #2842: with a `#[serde(rename_all = ...)]` container attribute (or field-level `rename`), `ActiveModel::from_json` stopped defaulting missing fields to `ActiveValue::NotSet` and instead errored with "missing field". Rebuilds the default-merge keyed off `col.json_key()` (which respects both `rename_all` and field-level `rename`) and the per-column dummy `ActiveValue`, instead of serializing a dummy model whose keys could mismatch. Closes #2854. Co-authored-by: Chris Tsang <chris.2y3@outlook.com>
1 parent 2f29543 commit 00f4602

4 files changed

Lines changed: 52 additions & 62 deletions

File tree

‎sea-orm-sync/src/entity/active_model.rs‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -474,7 +474,7 @@ pub trait ActiveModelTrait: Clone + Debug {
474474

475475
/// Create ActiveModel from a JSON value
476476
#[cfg(feature = "with-json")]
477-
fn from_json(mut json: serde_json::Value) -> Result<Self, DbErr>
477+
fn from_json(json: serde_json::Value) -> Result<Self, DbErr>
478478
where
479479
Self: crate::TryIntoModel<<Self::Entity as EntityTrait>::Model>,
480480
<<Self as ActiveModelTrait>::Entity as EntityTrait>::Model: IntoActiveModel<Self>,
@@ -483,43 +483,39 @@ pub trait ActiveModelTrait: Clone + Debug {
483483
{
484484
use crate::{IdenStatic, Iterable};
485485

486-
let serde_json::Value::Object(obj) = &json else {
486+
let serde_json::Value::Object(mut input) = json else {
487487
return Err(DbErr::Json(format!(
488488
"invalid type: expected JSON object for {}",
489489
<<Self as ActiveModelTrait>::Entity as IdenStatic>::as_str(&Default::default())
490490
)));
491491
};
492492

493+
let dummy_am = Self::default_values();
494+
let len = <<Self::Entity as EntityTrait>::Column>::iter().len();
493495
// Mark down which attribute exists in the JSON object
494-
let mut json_keys: Vec<(<Self::Entity as EntityTrait>::Column, bool)> = Vec::new();
496+
let mut json_keys = Vec::with_capacity(len);
497+
let mut merged = serde_json::Map::with_capacity(len);
495498

496499
for col in <<Self::Entity as EntityTrait>::Column>::iter() {
497500
let key = col.json_key();
498-
let has_key = obj.contains_key(key);
501+
let has_key = input.contains_key(key);
499502
json_keys.push((col, has_key));
500-
}
501-
502-
// Create dummy model with dummy values
503-
let dummy_model = Self::default_values();
504-
if let Ok(dummy_model) = dummy_model.try_into_model()
505-
&& let Ok(mut dummy_json) = serde_json::to_value(&dummy_model)
506-
{
507-
let serde_json::Value::Object(merged) = &mut dummy_json else {
508-
unreachable!();
509-
};
510-
let serde_json::Value::Object(obj) = json else {
511-
unreachable!();
512-
};
513-
// overwrite dummy values with input values
514-
for (key, value) in obj {
515-
merged.insert(key, value);
503+
match dummy_am.get(col) {
504+
ActiveValue::Unchanged(value) | ActiveValue::Set(value) => {
505+
merged.insert(key.to_owned(), sea_query::sea_value_to_json_value(&value));
506+
}
507+
_ => {}
516508
}
517-
json = dummy_json;
518509
}
519510

511+
merged.append(&mut input);
512+
let _ = input;
513+
514+
let json_value = serde_json::Value::Object(merged);
515+
520516
// Convert JSON object into ActiveModel via Model
521517
let model: <Self::Entity as EntityTrait>::Model =
522-
serde_json::from_value(json).map_err(json_err)?;
518+
serde_json::from_value(json_value).map_err(json_err)?;
523519
let mut am = model.into_active_model();
524520

525521
// Transform attribute that exists in JSON object into ActiveValue::Set, otherwise ActiveValue::NotSet

‎sea-orm-sync/src/tests_cfg/serde_rename.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ pub mod directional_rename_all {
9797
pub struct Model {
9898
#[sea_orm(primary_key)]
9999
pub id: i32,
100+
#[serde(rename = "user-name")]
100101
pub user_name: String,
101102
}
102103

@@ -123,7 +124,6 @@ mod tests {
123124

124125
// from_json uses camelCase keys
125126
let json = serde_json::json!({
126-
"id": 1,
127127
"firstName": "Max",
128128
"lastName": "Hermit",
129129
"email": "max@domain.com",
@@ -132,7 +132,8 @@ mod tests {
132132

133133
let am = ActiveModel::from_json(json).unwrap();
134134

135-
assert_eq!(am.id, ActiveValue::Set(1));
135+
// Test whether missing required fields can be handled correctly
136+
assert_eq!(am.id, ActiveValue::NotSet);
136137
assert_eq!(am.first_name, ActiveValue::Set("Max".to_string()));
137138
assert_eq!(am.last_name, ActiveValue::Set("Hermit".to_string()));
138139
assert_eq!(am.email, ActiveValue::Set("max@domain.com".to_string()));
@@ -155,7 +156,6 @@ mod tests {
155156

156157
// from_json uses deserialize names
157158
let json = serde_json::json!({
158-
"id": 1,
159159
"orderDate": "2024-01-01",
160160
"order-id": "ORD123",
161161
"serOnly": "ser-value",
@@ -164,7 +164,7 @@ mod tests {
164164

165165
let am = ActiveModel::from_json(json).unwrap();
166166

167-
assert_eq!(am.id, ActiveValue::Set(1));
167+
assert_eq!(am.id, ActiveValue::NotSet);
168168
assert_eq!(am.order_date, ActiveValue::Set("2024-01-01".to_string()));
169169
assert_eq!(am.order_id, ActiveValue::Set("ORD123".to_string()));
170170
assert_eq!(am.ser_only, ActiveValue::Set("ser-value".to_string()));
@@ -191,19 +191,18 @@ mod tests {
191191

192192
#[test]
193193
fn test_directional_rename_all() {
194-
use directional_rename_all::{ActiveModel, Column, Model};
194+
use directional_rename_all::{ActiveModel, Column};
195195

196196
assert_eq!(Column::Id.json_key(), "id");
197-
assert_eq!(Column::UserName.json_key(), "userName");
197+
assert_eq!(Column::UserName.json_key(), "user-name");
198198

199199
let json = serde_json::json!({
200-
"id": 1,
201-
"userName": "test_user"
200+
"user-name": "test_user"
202201
});
203202

204203
let am = ActiveModel::from_json(json).unwrap();
205204

206-
assert_eq!(am.id, ActiveValue::Set(1));
205+
assert_eq!(am.id, ActiveValue::NotSet);
207206
assert_eq!(am.user_name, ActiveValue::Set("test_user".to_string()));
208207
}
209208
}

‎src/entity/active_model.rs‎

Lines changed: 18 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ pub trait ActiveModelTrait: Clone + Debug {
482482

483483
/// Create ActiveModel from a JSON value
484484
#[cfg(feature = "with-json")]
485-
fn from_json(mut json: serde_json::Value) -> Result<Self, DbErr>
485+
fn from_json(json: serde_json::Value) -> Result<Self, DbErr>
486486
where
487487
Self: crate::TryIntoModel<<Self::Entity as EntityTrait>::Model>,
488488
<<Self as ActiveModelTrait>::Entity as EntityTrait>::Model: IntoActiveModel<Self>,
@@ -491,43 +491,39 @@ pub trait ActiveModelTrait: Clone + Debug {
491491
{
492492
use crate::{IdenStatic, Iterable};
493493

494-
let serde_json::Value::Object(obj) = &json else {
494+
let serde_json::Value::Object(mut input) = json else {
495495
return Err(DbErr::Json(format!(
496496
"invalid type: expected JSON object for {}",
497497
<<Self as ActiveModelTrait>::Entity as IdenStatic>::as_str(&Default::default())
498498
)));
499499
};
500500

501+
let dummy_am = Self::default_values();
502+
let len = <<Self::Entity as EntityTrait>::Column>::iter().len();
501503
// Mark down which attribute exists in the JSON object
502-
let mut json_keys: Vec<(<Self::Entity as EntityTrait>::Column, bool)> = Vec::new();
504+
let mut json_keys = Vec::with_capacity(len);
505+
let mut merged = serde_json::Map::with_capacity(len);
503506

504507
for col in <<Self::Entity as EntityTrait>::Column>::iter() {
505508
let key = col.json_key();
506-
let has_key = obj.contains_key(key);
509+
let has_key = input.contains_key(key);
507510
json_keys.push((col, has_key));
508-
}
509-
510-
// Create dummy model with dummy values
511-
let dummy_model = Self::default_values();
512-
if let Ok(dummy_model) = dummy_model.try_into_model()
513-
&& let Ok(mut dummy_json) = serde_json::to_value(&dummy_model)
514-
{
515-
let serde_json::Value::Object(merged) = &mut dummy_json else {
516-
unreachable!();
517-
};
518-
let serde_json::Value::Object(obj) = json else {
519-
unreachable!();
520-
};
521-
// overwrite dummy values with input values
522-
for (key, value) in obj {
523-
merged.insert(key, value);
511+
match dummy_am.get(col) {
512+
ActiveValue::Unchanged(value) | ActiveValue::Set(value) => {
513+
merged.insert(key.to_owned(), sea_query::sea_value_to_json_value(&value));
514+
}
515+
_ => {}
524516
}
525-
json = dummy_json;
526517
}
527518

519+
merged.append(&mut input);
520+
let _ = input;
521+
522+
let json_value = serde_json::Value::Object(merged);
523+
528524
// Convert JSON object into ActiveModel via Model
529525
let model: <Self::Entity as EntityTrait>::Model =
530-
serde_json::from_value(json).map_err(json_err)?;
526+
serde_json::from_value(json_value).map_err(json_err)?;
531527
let mut am = model.into_active_model();
532528

533529
// Transform attribute that exists in JSON object into ActiveValue::Set, otherwise ActiveValue::NotSet

‎src/tests_cfg/serde_rename.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,7 @@ pub mod directional_rename_all {
9797
pub struct Model {
9898
#[sea_orm(primary_key)]
9999
pub id: i32,
100+
#[serde(rename = "user-name")]
100101
pub user_name: String,
101102
}
102103

@@ -123,7 +124,6 @@ mod tests {
123124

124125
// from_json uses camelCase keys
125126
let json = serde_json::json!({
126-
"id": 1,
127127
"firstName": "Max",
128128
"lastName": "Hermit",
129129
"email": "max@domain.com",
@@ -132,7 +132,8 @@ mod tests {
132132

133133
let am = ActiveModel::from_json(json).unwrap();
134134

135-
assert_eq!(am.id, ActiveValue::Set(1));
135+
// Test whether missing required fields can be handled correctly
136+
assert_eq!(am.id, ActiveValue::NotSet);
136137
assert_eq!(am.first_name, ActiveValue::Set("Max".to_string()));
137138
assert_eq!(am.last_name, ActiveValue::Set("Hermit".to_string()));
138139
assert_eq!(am.email, ActiveValue::Set("max@domain.com".to_string()));
@@ -155,7 +156,6 @@ mod tests {
155156

156157
// from_json uses deserialize names
157158
let json = serde_json::json!({
158-
"id": 1,
159159
"orderDate": "2024-01-01",
160160
"order-id": "ORD123",
161161
"serOnly": "ser-value",
@@ -164,7 +164,7 @@ mod tests {
164164

165165
let am = ActiveModel::from_json(json).unwrap();
166166

167-
assert_eq!(am.id, ActiveValue::Set(1));
167+
assert_eq!(am.id, ActiveValue::NotSet);
168168
assert_eq!(am.order_date, ActiveValue::Set("2024-01-01".to_string()));
169169
assert_eq!(am.order_id, ActiveValue::Set("ORD123".to_string()));
170170
assert_eq!(am.ser_only, ActiveValue::Set("ser-value".to_string()));
@@ -191,19 +191,18 @@ mod tests {
191191

192192
#[test]
193193
fn test_directional_rename_all() {
194-
use directional_rename_all::{ActiveModel, Column, Model};
194+
use directional_rename_all::{ActiveModel, Column};
195195

196196
assert_eq!(Column::Id.json_key(), "id");
197-
assert_eq!(Column::UserName.json_key(), "userName");
197+
assert_eq!(Column::UserName.json_key(), "user-name");
198198

199199
let json = serde_json::json!({
200-
"id": 1,
201-
"userName": "test_user"
200+
"user-name": "test_user"
202201
});
203202

204203
let am = ActiveModel::from_json(json).unwrap();
205204

206-
assert_eq!(am.id, ActiveValue::Set(1));
205+
assert_eq!(am.id, ActiveValue::NotSet);
207206
assert_eq!(am.user_name, ActiveValue::Set("test_user".to_string()));
208207
}
209208
}

0 commit comments

Comments
 (0)