Radish alpha
h
Radicle Heartwood Protocol & Stack
Radicle
Git (anonymous pull)
Log in to clone via SSH
REVIEW: Refactor `Payload` to `PayloadUpsert`
Lorenz Leutgeb committed 6 months ago
commit db4946dfe22ba583659575e5418edee32b0da19d
parent bcd23d68ba9d44f4d9e67d33a2bf02e4c47245dd
2 files changed +30 -21
modified crates/radicle-cli/src/commands/id.rs
@@ -163,7 +163,7 @@ pub fn run(args: Args, ctx: impl term::Context) -> anyhow::Result<()> {

                // TODO(erikli): whenever `clap` starts supporting custom value parsers
                // for a series of values, we can parse into `Payload` implicitly.
-
                let payloads = args::Payload::try_parse_many(&payload)
+
                let payloads = args::PayloadUpsert::parse_many(&payload)
                    .map_ok(|p| (p.id, p.key, p.value))
                    .collect::<Result<Vec<_>, _>>()?;

modified crates/radicle-cli/src/commands/id/args.rs
@@ -25,7 +25,7 @@ See the rad-id(1) man page for more information.
"#;

#[derive(Debug, Error)]
-
pub enum PayloadParseError {
+
pub enum PayloadUpsertParseError {
    #[error("could not parse payload id: {0}")]
    IdParse(#[from] TypeNameParse),
    #[error("could not parse json value: {0}")]
@@ -33,27 +33,36 @@ pub enum PayloadParseError {
}

#[derive(Clone, Debug)]
-
pub struct Payload {
+
pub struct PayloadUpsert {
    pub(super) id: PayloadId,
    pub(super) key: String,
    pub(super) value: json::Value,
}

-
impl Payload {
-
    /// Parses the list of all payload values that were aggregated by `clap`.
-
    /// E.g. `--payload key name value --payload key name value` will result
-
    /// in `["key","name","value","key","name","value"]`.
-
    pub(super) fn try_parse_many(
+
impl PayloadUpsert {
+
    /// Parses a slice of all payload upserts as aggregated by `clap`
+
    /// (see [`Command::Update::payload`]).
+
    /// E.g. `["com.example.one", "name", "1", "com.example.two", "name2", "2"]`
+
    /// will result in iterator over two [`PayloadUpsert`]s.
+
    /// 
+
    /// # Panics
+
    /// 
+
    /// If the length of `values` is not divisible by 3.
+
    /// (To catch errors in the definition of the parser derived from
+
    /// [`Command::Update`] or `clap` itself, and unexpected changes to
+
    /// `clap`s behaviour in the future.)
+
    pub(super) fn parse_many(
        values: &[String],
-
    ) -> impl Iterator<Item = Result<Self, PayloadParseError>> + use<'_> {
-
        // `clap` makes sure we don't have 3 values per option occurence, so we can just
-
        // chunk the aggregated list
-
        debug_assert!(
-
            values.len() % 3 == 0,
-
            "number of values should be divsible by 3"
-
        );
-
        values.chunks(3).map(|chunk| {
-
            Ok(Payload {
+
    ) -> impl Iterator<Item = Result<Self, PayloadUpsertParseError>> + use<'_> {
+
        // `clap` ensures we have 3 values per option occurrence,
+
        // so we can chunk the aggregated slice exactly.
+
        let chunks = values.chunks_exact(3);
+

+
        assert!(chunks.remainder().is_empty());
+

+
        chunks.map(|chunk| {
+
            // Slice accesses will not panic, guaranteed by `chunks_exact(3)`.
+
            Ok(PayloadUpsert {
                id: PayloadId::from_str(&chunk[0])?,
                key: chunk[1].to_owned(),
                value: json::from_str(&chunk[2].to_owned())?,
@@ -232,7 +241,7 @@ pub(super) enum Command {

#[cfg(test)]
mod test {
-
    use super::{Args, Payload};
+
    use super::{Args, PayloadUpsert};
    use clap::error::ErrorKind;
    use clap::Parser;

@@ -301,7 +310,7 @@ mod test {

    #[test]
    fn should_parse_into_payload() {
-
        let payload: Result<Vec<_>, _> = Payload::try_parse_many(&[
+
        let payload: Result<Vec<_>, _> = PayloadUpsert::parse_many(&[
            "xyz.radicle.project".to_string(),
            "name".to_string(),
            "{}".to_string(),
@@ -311,10 +320,10 @@ mod test {
    }

    #[test]
-
    #[should_panic(expected = "number of values should be divsible by 3")]
+
    #[should_panic(expected = "assertion failed: chunks.remainder().is_empty()")]
    fn should_not_parse_into_payload() {
        let _: Result<Vec<_>, _> =
-
            Payload::try_parse_many(&["xyz.radicle.project".to_string(), "name".to_string()])
+
            PayloadUpsert::parse_many(&["xyz.radicle.project".to_string(), "name".to_string()])
                .collect();
    }
}