From e07b62d74a451a0ba6617f1bc7875d9c71c85ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=81abor?= Date: Sun, 2 Oct 2022 21:50:51 +0200 Subject: [PATCH] fix(serde_v8): Implement MapAccess for StructAccess (#15962) `StructAccess` implements` serde::de::SeqAccess` instead of `serde::de::MapAccess` thus interpreting structs as sequences. --- serde_v8/de.rs | 93 ++++++++++++++++++++++++++++---------------- serde_v8/tests/de.rs | 59 +++++++++++++++++++++++++++- 2 files changed, 117 insertions(+), 35 deletions(-) diff --git a/serde_v8/de.rs b/serde_v8/de.rs index 16a8a887c8..79fb9ae6d5 100644 --- a/serde_v8/de.rs +++ b/serde_v8/de.rs @@ -372,14 +372,32 @@ impl<'de, 'a, 'b, 's, 'x> de::Deserializer<'de> } _ => { // Regular struct - let obj = self.input.try_into().or(Err(Error::ExpectedObject))?; - visitor.visit_seq(StructAccess { - fields, - obj, - pos: 0, - scope: self.scope, - _cache: None, - }) + let obj: v8::Local = + self.input.try_into().or(Err(Error::ExpectedObject))?; + + // Fields names are a hint and must be inferred when not provided + if fields.is_empty() { + let prop_names = + obj.get_own_property_names(self.scope, Default::default()); + let keys: Vec = match prop_names { + Some(names) => from_v8(self.scope, names.into()).unwrap(), + None => vec![], + }; + + visitor.visit_map(MapObjectAccess { + obj, + scope: self.scope, + keys: keys.into_iter(), + next_value: None, + }) + } else { + visitor.visit_map(StructAccess { + obj, + scope: self.scope, + keys: fields.iter(), + next_value: None, + }) + } } } } @@ -489,8 +507,7 @@ impl<'de> de::MapAccess<'de> for MapObjectAccess<'_, '_> { } self.next_value = Some(v8_val); let mut deserializer = Deserializer::new(self.scope, key.v8_value, None); - let k = seed.deserialize(&mut deserializer)?; - return Ok(Some(k)); + return seed.deserialize(&mut deserializer).map(Some); } Ok(None) } @@ -499,7 +516,10 @@ impl<'de> de::MapAccess<'de> for MapObjectAccess<'_, '_> { &mut self, seed: V, ) -> Result { - let v8_val = self.next_value.take().unwrap(); + let v8_val = self + .next_value + .take() + .expect("Call next_key_seed before next_value_seed"); let mut deserializer = Deserializer::new(self.scope, v8_val, None); seed.deserialize(&mut deserializer) } @@ -553,36 +573,41 @@ impl<'de> de::MapAccess<'de> for MapPairsAccess<'_, '_> { struct StructAccess<'a, 'b, 's> { obj: v8::Local<'a, v8::Object>, scope: &'b mut v8::HandleScope<'s>, - fields: &'static [&'static str], - pos: usize, - _cache: Option<&'b mut KeyCache>, + keys: std::slice::Iter<'static, &'static str>, + next_value: Option>, } -impl<'de> de::SeqAccess<'de> for StructAccess<'_, '_, '_> { +impl<'de> de::MapAccess<'de> for StructAccess<'_, '_, '_> { type Error = Error; - fn next_element_seed>( - &mut self, - seed: T, - ) -> Result> { - if self.pos >= self.fields.len() { - return Ok(None); + fn next_key_seed(&mut self, seed: K) -> Result> + where + K: de::DeserializeSeed<'de>, + { + for field in self.keys.by_ref() { + let key = v8_struct_key(self.scope, field).into(); + let val = self.obj.get(self.scope, key).unwrap(); + if val.is_undefined() { + // Historically keys/value pairs with undefined values are not added to the output + continue; + } + self.next_value = Some(val); + let mut deserializer = Deserializer::new(self.scope, key, None); + return seed.deserialize(&mut deserializer).map(Some); } + Ok(None) + } - let pos = self.pos; - self.pos += 1; - - let field = self.fields[pos]; - let key = v8_struct_key(self.scope, field).into(); - let val = self.obj.get(self.scope, key).unwrap(); + fn next_value_seed(&mut self, seed: V) -> Result + where + V: de::DeserializeSeed<'de>, + { + let val = self + .next_value + .take() + .expect("Call next_key_seed before next_value_seed"); let mut deserializer = Deserializer::new(self.scope, val, None); - match seed.deserialize(&mut deserializer) { - Ok(val) => Ok(Some(val)), - // Fallback to Ok(None) for #[serde(Default)] at cost of error quality ... - // TODO(@AaronO): double check that there's no better solution - Err(_) if val.is_undefined() => Ok(None), - Err(e) => Err(e), - } + seed.deserialize(&mut deserializer) } } diff --git a/serde_v8/tests/de.rs b/serde_v8/tests/de.rs index c29b357603..7affe5f8ec 100644 --- a/serde_v8/tests/de.rs +++ b/serde_v8/tests/de.rs @@ -1,5 +1,5 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. -use serde::Deserialize; +use serde::{Deserialize, Deserializer}; use serde_v8::utils::{js_exec, v8_do}; use serde_v8::ByteString; @@ -233,6 +233,63 @@ fn de_buffers() { ); } +// Structs +#[derive(Debug, PartialEq, Deserialize)] +struct StructUnit; + +#[derive(Debug, PartialEq)] +struct StructPayload { + a: u64, + b: u64, +} + +struct StructVisitor; + +impl<'de> serde::de::Visitor<'de> for StructVisitor { + type Value = StructPayload; + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + formatter.write_str("struct StructPayload") + } + fn visit_map(self, mut map: A) -> Result + where + A: serde::de::MapAccess<'de>, + { + let mut payload = StructPayload { a: 0, b: 0 }; + while let Some(key) = map.next_key::()? { + match key.as_ref() { + "a" => payload.a = map.next_value()?, + "b" => payload.b = map.next_value()?, + f => panic!("Unknown field {}", f), + } + } + Ok(payload) + } +} + +detest!(de_unit_struct, StructUnit, "'StructUnit'", StructUnit); + +#[test] +fn de_struct() { + dedo("({ a: 1, b: 2 })", |scope, v| { + let mut de = serde_v8::Deserializer::new(scope, v, None); + let payload = de + .deserialize_struct("StructPayload", &[], StructVisitor) + .unwrap(); + assert_eq!(payload, StructPayload { a: 1, b: 2 }) + }) +} + +#[test] +fn de_struct_hint() { + dedo("({ a: 1, b: 2 })", |scope, v| { + let mut de = serde_v8::Deserializer::new(scope, v, None); + let payload = de + .deserialize_struct("StructPayload", &["a", "b"], StructVisitor) + .unwrap(); + assert_eq!(payload, StructPayload { a: 1, b: 2 }) + }) +} + //// // JSON tests: serde_json::Value compatibility ////