Implement De-/Serialize for Record manually (#12365)

# Description
This decouples the serialized representation of `Record` from its
internal implementation. It now gets treated as a map type in `serde`.

This has several benefits:
- more efficient representation (not showing inner fields)
- human readable e.g. as a JSON record
- no breaking changes when refactoring the `Record` internals in the
future (see #12326, or potential introduction of `indexmap::IndexMap`
for large N)
- we now deny the creation of invalid records a non-cooperating plugin
could produce
  - guaranteed key-value correspondence
  - checking for unique keys

# Breaking change to the plugin protocol:
Now expects a record/map directly as the `Record.val` field instead of a
serialization of it.
This commit is contained in:
Stefan Holderbach 2024-04-07 01:21:03 +02:00 committed by GitHub
parent db1dccc762
commit 01c79bbefc
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

View File

@ -2,9 +2,9 @@ use std::ops::RangeBounds;
use crate::{ShellError, Span, Value};
use serde::{Deserialize, Serialize};
use serde::{de::Visitor, ser::SerializeMap, Deserialize, Serialize};
#[derive(Debug, Clone, Default, Serialize, Deserialize)]
#[derive(Debug, Clone, Default)]
pub struct Record {
inner: Vec<(String, Value)>,
}
@ -285,6 +285,78 @@ impl Record {
}
}
impl Serialize for Record {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
let mut map = serializer.serialize_map(Some(self.len()))?;
for (k, v) in self {
map.serialize_entry(k, v)?;
}
map.end()
}
}
impl<'de> Deserialize<'de> for Record {
/// Special deserialization implementation that turns a map-pattern into a [`Record`]
///
/// Denies duplicate keys
///
/// ```rust
/// use serde_json::{from_str, Result};
/// use nu_protocol::{Record, Value, record};
///
/// // A `Record` in json is a Record with a packed `Value`
/// // The `Value` record has a single key indicating its type and the inner record describing
/// // its representation of value and the associated `Span`
/// let ok = r#"{"a": {"Int": {"val": 42, "span": {"start": 0, "end": 0}}},
/// "b": {"Int": {"val": 37, "span": {"start": 0, "end": 0}}}}"#;
/// let ok_rec: Record = from_str(ok).unwrap();
/// assert_eq!(Value::test_record(ok_rec),
/// Value::test_record(record!{"a" => Value::test_int(42),
/// "b" => Value::test_int(37)}));
/// // A repeated key will lead to a deserialization error
/// let bad = r#"{"a": {"Int": {"val": 42, "span": {"start": 0, "end": 0}}},
/// "a": {"Int": {"val": 37, "span": {"start": 0, "end": 0}}}}"#;
/// let bad_rec: Result<Record> = from_str(bad);
/// assert!(bad_rec.is_err());
/// ```
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
deserializer.deserialize_map(RecordVisitor)
}
}
struct RecordVisitor;
impl<'de> Visitor<'de> for RecordVisitor {
type Value = Record;
fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result {
formatter.write_str("a nushell `Record` mapping string keys/columns to nushell `Value`")
}
fn visit_map<A>(self, mut map: A) -> Result<Self::Value, A::Error>
where
A: serde::de::MapAccess<'de>,
{
let mut record = Record::with_capacity(map.size_hint().unwrap_or(0));
while let Some((key, value)) = map.next_entry::<String, Value>()? {
if record.insert(key, value).is_some() {
return Err(serde::de::Error::custom(
"invalid entry, duplicate keys are not allowed for `Record`",
));
}
}
Ok(record)
}
}
impl FromIterator<(String, Value)> for Record {
fn from_iter<T: IntoIterator<Item = (String, Value)>>(iter: T) -> Self {
// TODO: should this check for duplicate keys/columns?