Skip to content

Commit

Permalink
Fix serializer and deserializer not symmetric
Browse files Browse the repository at this point in the history
Some optional types encode to null but don't parse null.
- Block.last_commit
- CommitSig.validator_address
  • Loading branch information
yihuang committed May 1, 2020
1 parent e8e0179 commit e81e98a
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 40 deletions.
21 changes: 12 additions & 9 deletions tendermint/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,18 @@ where
pub signatures: Option<CommitSigs>,
}

let commit = TmpCommit::deserialize(deserializer)?;
if commit.block_id.is_none() || commit.signatures.is_none() {
Ok(None)
if let Some(commit) = <Option<TmpCommit>>::deserialize(deserializer)? {
if let Some(block_id) = commit.block_id {
Ok(Some(Commit {
height: commit.height,
round: commit.round,
block_id,
signatures: commit.signatures.unwrap_or_else(|| CommitSigs::new(vec![])),
}))
} else {
Ok(None)
}
} else {
Ok(Some(Commit {
height: commit.height,
round: commit.round,
block_id: commit.block_id.unwrap(),
signatures: commit.signatures.unwrap(),
}))
Ok(None)
}
}
67 changes: 37 additions & 30 deletions tendermint/src/serializers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,45 +169,52 @@ where
hash: String,
parts: Parts,
}
let tmp_id = BlockId::deserialize(deserializer)?;
if tmp_id.hash.is_empty() {
Ok(None)
if let Some(tmp_id) = <Option<BlockId>>::deserialize(deserializer)? {
if tmp_id.hash.is_empty() {
Ok(None)
} else {
Ok(Some(block::Id {
hash: Hash::from_str(&tmp_id.hash)
.map_err(|err| D::Error::custom(format!("{}", err)))?,
parts: if tmp_id.parts.hash.is_empty() {
None
} else {
Some(block::parts::Header {
total: tmp_id.parts.total,
hash: Hash::from_str(&tmp_id.parts.hash)
.map_err(|err| D::Error::custom(format!("{}", err)))?,
})
},
}))
}
} else {
Ok(Some(block::Id {
hash: Hash::from_str(&tmp_id.hash)
.map_err(|err| D::Error::custom(format!("{}", err)))?,
parts: if tmp_id.parts.hash.is_empty() {
None
} else {
Some(block::parts::Header {
total: tmp_id.parts.total,
hash: Hash::from_str(&tmp_id.parts.hash)
.map_err(|err| D::Error::custom(format!("{}", err)))?,
})
},
}))
Ok(None)
}
}

pub(crate) fn parse_non_empty_id<'de, D>(deserializer: D) -> Result<Option<Id>, D::Error>
where
D: Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
if s.is_empty() {
Ok(None)
} else {
// TODO: how can we avoid rewriting code here?
match Id::from_str(&s).map_err(|_| {
D::Error::custom(format!(
"expected {}-character hex string, got {:?}",
LENGTH * 2,
s
))
}) {
Ok(id) => Ok(Option::from(id)),
Err(_) => Ok(None),
let s = <Option<String>>::deserialize(deserializer)?;
if let Some(s) = s {
if s.is_empty() {
Ok(None)
} else {
// TODO: how can we avoid rewriting code here?
match Id::from_str(&s).map_err(|_| {
D::Error::custom(format!(
"expected {}-character hex string, got {:?}",
LENGTH * 2,
s
))
}) {
Ok(id) => Ok(Option::from(id)),
Err(_) => Ok(None),
}
}
} else {
Ok(None)
}
}

Expand Down
3 changes: 2 additions & 1 deletion tendermint/tests/lite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,8 @@ fn run_test_cases(cases: TestCases) {
}

fn check_symmetric_encoding<T: Serialize + for<'a> Deserialize<'a>>(value: &T) {
serde_json::from_str::<T>(&serde_json::to_string(value).unwrap()).expect("encoded can be decode again");
serde_json::from_str::<T>(&serde_json::to_string(value).unwrap())
.expect("encoded can be decode again");
}

// Link to the commit where the happy_path.json was created:
Expand Down

0 comments on commit e81e98a

Please sign in to comment.