From db7f2d40e32ef0fac7425054c4c49e22cb8bc70f Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 8 Dec 2022 15:20:11 +0000 Subject: [PATCH] git-ref-format: fix to deserialization When deserializing through `serde_json::from_value` the following error would occur: panicked at 'called Result::unwrap() on an Err value: Error("invalid type: string "master", expected a borrowed string", line: 0, column: 0)' To fix this it is necessary for the Deserialize implementations to deserialize into an owned String and convert from there. While attempting to test this the `Qualified` and `Namespaced` implementations were not able to be used: error: implementation of `serde::de::Deserialize` is not general enough --> git-ref-format/t/src/properties/name.rs:86:9 | 86 | roundtrip::json_value(qualified); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ implementation of `serde::de::Deserialize` is not general enough | = note: `Qualified<'_>` must implement `serde::de::Deserialize<'0>`, for any lifetime `'0`... = note: ...but `Qualified<'_>` actually implements `serde::de::Deserialize<'1>`, for some specific lifetime `'1` The Deserialize for both of those types now return owned data by returning a 'static lifetime instead. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- git-ref-format/core/src/serde.rs | 18 ++++++++++++------ git-ref-format/t/Cargo.toml | 1 + git-ref-format/t/src/properties/name.rs | 19 +++++++++++++++++-- git-ref-format/t/src/properties/pattern.rs | 5 +++++ 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/git-ref-format/core/src/serde.rs b/git-ref-format/core/src/serde.rs index 47408c5..f492695 100644 --- a/git-ref-format/core/src/serde.rs +++ b/git-ref-format/core/src/serde.rs @@ -42,7 +42,8 @@ impl<'de> Deserialize<'de> for RefString { where D: Deserializer<'de>, { - Deserialize::deserialize(deserializer).map(|x: &RefStr| x.to_owned()) + Deserialize::deserialize(deserializer) + .and_then(|x: String| Self::try_from(x).map_err(de::Error::custom)) } } @@ -83,7 +84,8 @@ impl<'de> Deserialize<'de> for PatternString { where D: Deserializer<'de>, { - Deserialize::deserialize(deserializer).map(|x: &PatternStr| x.to_owned()) + Deserialize::deserialize(deserializer) + .and_then(|x: String| Self::try_from(x).map_err(de::Error::custom)) } } @@ -97,15 +99,17 @@ impl Serialize for PatternString { } } -impl<'de: 'a, 'a> Deserialize<'de> for Qualified<'a> { +impl<'de> Deserialize<'de> for Qualified<'static> { #[inline] fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { - Deserialize::deserialize(deserializer).and_then(|s: &RefStr| { + Deserialize::deserialize(deserializer).and_then(|s: String| { + let s = RefString::try_from(s).map_err(de::Error::custom)?; s.qualified() .ok_or_else(|| de::Error::custom("not a qualified ref")) + .map(|q| q.into_owned()) }) } } @@ -120,15 +124,17 @@ impl Serialize for Qualified<'_> { } } -impl<'de: 'a, 'a> Deserialize<'de> for Namespaced<'a> { +impl<'de> Deserialize<'de> for Namespaced<'static> { #[inline] fn deserialize(deserializer: D) -> Result where D: Deserializer<'de>, { - Deserialize::deserialize(deserializer).and_then(|s: &RefStr| { + Deserialize::deserialize(deserializer).and_then(|s: String| { + let s = RefString::try_from(s).map_err(de::Error::custom)?; s.to_namespaced() .ok_or_else(|| de::Error::custom("not a namespaced ref")) + .map(|ns| ns.into_owned()) }) } } diff --git a/git-ref-format/t/Cargo.toml b/git-ref-format/t/Cargo.toml index df46ea7..d83a201 100644 --- a/git-ref-format/t/Cargo.toml +++ b/git-ref-format/t/Cargo.toml @@ -19,6 +19,7 @@ proptest = "1" [dev-dependencies] assert_matches = "1.5" +serde_json = "1" [dev-dependencies.git-ref-format] path = ".." diff --git a/git-ref-format/t/src/properties/name.rs b/git-ref-format/t/src/properties/name.rs index b7d9318..2a9b429 100644 --- a/git-ref-format/t/src/properties/name.rs +++ b/git-ref-format/t/src/properties/name.rs @@ -3,7 +3,7 @@ use std::convert::TryFrom; -use git_ref_format::{Error, RefStr, RefString}; +use git_ref_format::{name, refname, Error, RefStr, RefString}; use proptest::prelude::*; use test_helpers::roundtrip; @@ -75,7 +75,22 @@ proptest! { #[test] fn json(input in gen::valid()) { - roundtrip::json(RefString::try_from(input).unwrap()) + let input = RefString::try_from(input).unwrap(); + roundtrip::json(input.clone()); + let qualified = refname!("refs/heads").and(input).qualified().unwrap().into_owned(); + roundtrip::json(qualified.clone()); + let namespaced = qualified.with_namespace(name::component!("foo")); + roundtrip::json(namespaced); + } + + #[test] + fn json_value(input in gen::valid()) { + let input = RefString::try_from(input).unwrap(); + roundtrip::json_value(input.clone()); + let qualified = refname!("refs/heads").and(input).qualified().unwrap().into_owned(); + roundtrip::json_value(qualified.clone()); + let namespaced = qualified.with_namespace(name::component!("foo")); + roundtrip::json_value(namespaced); } #[test] diff --git a/git-ref-format/t/src/properties/pattern.rs b/git-ref-format/t/src/properties/pattern.rs index 4e36d62..6348af8 100644 --- a/git-ref-format/t/src/properties/pattern.rs +++ b/git-ref-format/t/src/properties/pattern.rs @@ -48,6 +48,11 @@ proptest! { roundtrip::json(refspec::PatternString::try_from(input).unwrap()) } + #[test] + fn json_value(input in gen::with_glob()) { + roundtrip::json_value(refspec::PatternString::try_from(input).unwrap()) + } + #[test] fn cbor(input in gen::with_glob()) { roundtrip::cbor(refspec::PatternString::try_from(input).unwrap())