From 1d7478cd909776b4423a90cd0bf40c10d711e989 Mon Sep 17 00:00:00 2001 From: Sebastian Martinez Date: Mon, 18 Aug 2025 15:08:43 +0200 Subject: [PATCH] radicle: introduce `cob::common::Title` This patch adds the `cob::common::Title` struct, this allows instead of using `std::str::String` or generics and traits for it, to define more precisely what a title should be. It trims the provided string and makes sure it contains no carriage return or new line characters. Where a `std::str::String` makes more sense so it's easier to mutate it, we keep the code as is. Co-authored-by: Fintan Halpenny --- crates/radicle-cli/src/commands/id.rs | 18 +-- crates/radicle-cli/src/commands/inbox.rs | 4 +- crates/radicle-cli/src/commands/issue.rs | 19 +-- crates/radicle-cli/src/commands/patch/edit.rs | 20 +-- crates/radicle-cli/src/commands/publish.rs | 11 +- crates/radicle-cli/src/terminal/issue.rs | 4 +- crates/radicle-cli/src/terminal/patch.rs | 38 +++--- crates/radicle-cli/tests/commands.rs | 5 +- crates/radicle-node/src/test/node.rs | 2 +- crates/radicle-node/src/tests.rs | 10 +- crates/radicle-node/src/tests/e2e.rs | 35 ++++-- crates/radicle-remote-helper/src/push.rs | 4 +- crates/radicle/src/cob/common.rs | 59 +++++++++ crates/radicle/src/cob/identity.rs | 118 ++++++++++++------ crates/radicle/src/cob/issue.rs | 92 ++++++++------ crates/radicle/src/cob/issue/cache.rs | 2 +- crates/radicle/src/cob/patch.rs | 60 ++++----- crates/radicle/src/cob/patch/cache.rs | 69 +++++++--- crates/radicle/src/cob/test.rs | 6 +- crates/radicle/src/storage/refs.rs | 6 +- 20 files changed, 389 insertions(+), 193 deletions(-) diff --git a/crates/radicle-cli/src/commands/id.rs b/crates/radicle-cli/src/commands/id.rs index 166b0d59..93f3cf12 100644 --- a/crates/radicle-cli/src/commands/id.rs +++ b/crates/radicle-cli/src/commands/id.rs @@ -4,6 +4,7 @@ use std::{ffi::OsString, io}; use anyhow::{anyhow, Context}; use radicle::cob::identity::{self, IdentityMut, Revision, RevisionId}; +use radicle::cob::Title; use radicle::identity::doc::update; use radicle::identity::doc::update::EditVisibility; use radicle::identity::{doc, Doc, Identity, RawDoc}; @@ -56,7 +57,7 @@ Options #[derive(Clone, Debug, Default)] pub enum Operation { Update { - title: Option, + title: Option, description: Option<String>, delegate: Vec<Did>, rescind: Vec<Did>, @@ -75,7 +76,7 @@ pub enum Operation { }, EditRevision { revision: Rev, - title: Option<String>, + title: Option<Title>, description: Option<String>, }, RedactRevision { @@ -115,7 +116,7 @@ impl Args for Options { let mut op: Option<OperationName> = None; let mut revision: Option<Rev> = None; let mut rid: Option<RepoId> = None; - let mut title: Option<String> = None; + let mut title: Option<Title> = None; let mut description: Option<String> = None; let mut delegate: Vec<Did> = Vec::new(); let mut rescind: Vec<Did> = Vec::new(); @@ -139,7 +140,8 @@ impl Args for Options { Long("title") if op == Some(OperationName::Edit) || op == Some(OperationName::Update) => { - title = Some(parser.value()?.to_string_lossy().into()); + let val = parser.value()?; + title = Some(term::args::string(&val).try_into()?); } Long("description") if op == Some(OperationName::Edit) || op == Some(OperationName::Update) => @@ -536,7 +538,7 @@ fn print_meta(revision: &Revision, previous: &Doc, profile: &Profile) -> anyhow: attrs.push([ term::format::bold("Title").into(), - term::label(revision.title.to_owned()), + term::label(revision.title.to_string()), ]); attrs.push([ term::format::bold("Revision").into(), @@ -632,9 +634,9 @@ fn print( } fn edit_title_description( - title: Option<String>, + title: Option<Title>, description: Option<String>, -) -> anyhow::Result<Option<(String, String)>> { +) -> anyhow::Result<Option<(Title, String)>> { const HELP: &str = r#"<!-- Please enter a patch message for your changes. An empty message aborts the patch proposal. @@ -659,7 +661,7 @@ and description. } fn update<R, G>( - title: Option<String>, + title: Option<Title>, description: Option<String>, doc: Doc, current: &mut IdentityMut<R>, diff --git a/crates/radicle-cli/src/commands/inbox.rs b/crates/radicle-cli/src/commands/inbox.rs index 70bf5144..25ba612b 100644 --- a/crates/radicle-cli/src/commands/inbox.rs +++ b/crates/radicle-cli/src/commands/inbox.rs @@ -365,7 +365,7 @@ impl NotificationRow { ) -> Self { Self { category: term::format::dim(category), - summary: term::Paint::new(summary), + summary: term::Paint::new(summary.to_string()), state, name: term::format::tertiary(name), } @@ -456,7 +456,7 @@ impl NotificationRow { }; ( String::from("id"), - rev.title.clone(), + rev.title.to_string(), term::format::identity::state(&rev.state), ) } else { diff --git a/crates/radicle-cli/src/commands/issue.rs b/crates/radicle-cli/src/commands/issue.rs index bf40bab2..8465cbae 100644 --- a/crates/radicle-cli/src/commands/issue.rs +++ b/crates/radicle-cli/src/commands/issue.rs @@ -9,7 +9,7 @@ use anyhow::{anyhow, Context as _}; use radicle::cob::common::{Label, Reaction}; use radicle::cob::issue::{CloseReason, State}; -use radicle::cob::{issue, thread}; +use radicle::cob::{issue, thread, Title}; use radicle::crypto; use radicle::git::Oid; use radicle::issue::cache::Issues as _; @@ -107,11 +107,11 @@ pub enum Assigned { pub enum Operation { Edit { id: Rev, - title: Option<String>, + title: Option<Title>, description: Option<String>, }, Open { - title: Option<String>, + title: Option<Title>, description: Option<String>, labels: Vec<Label>, assignees: Vec<Did>, @@ -189,7 +189,7 @@ impl Args for Options { let mut op: Option<OperationName> = None; let mut id: Option<Rev> = None; let mut assigned: Option<Assigned> = None; - let mut title: Option<String> = None; + let mut title: Option<Title> = None; let mut reaction: Option<Reaction> = None; let mut comment_id: Option<thread::CommentId> = None; let mut description: Option<String> = None; @@ -236,7 +236,8 @@ impl Args for Options { Long("title") if op == Some(OperationName::Open) || op == Some(OperationName::Edit) => { - title = Some(parser.value()?.to_string_lossy().into()); + let val = parser.value()?; + title = Some(term::args::string(&val).try_into()?); } Long("description") if op == Some(OperationName::Open) || op == Some(OperationName::Edit) => @@ -828,7 +829,7 @@ where } fn open<R, G>( - title: Option<String>, + title: Option<Title>, description: Option<String>, labels: Vec<Label>, assignees: Vec<Did>, @@ -849,7 +850,7 @@ where anyhow::bail!("aborting issue creation due to empty title or description"); }; let issue = cache.create( - &title, + title, description, labels.as_slice(), assignees.as_slice(), @@ -867,7 +868,7 @@ fn edit<'a, 'g, R, G>( issues: &'g mut issue::Cache<issue::Issues<'a, R>, cob::cache::StoreWriter>, repo: &storage::git::Repository, id: Rev, - title: Option<String>, + title: Option<Title>, description: Option<String>, signer: &Device<G>, ) -> anyhow::Result<issue::IssueMut<'a, 'g, R, cob::cache::StoreWriter>> @@ -896,7 +897,7 @@ where // Editing via the editor. let Some((title, description)) = term::issue::get_title_description( - Some(title.unwrap_or(issue.title().to_owned())), + title.and(Title::new(issue.title()).ok()), Some(description.unwrap_or(issue.description().to_owned())), )? else { diff --git a/crates/radicle-cli/src/commands/patch/edit.rs b/crates/radicle-cli/src/commands/patch/edit.rs index 70ec695f..d7e4ccff 100644 --- a/crates/radicle-cli/src/commands/patch/edit.rs +++ b/crates/radicle-cli/src/commands/patch/edit.rs @@ -2,6 +2,7 @@ use super::*; use radicle::cob; use radicle::cob::patch; +use radicle::cob::Title; use radicle::crypto; use radicle::node::device::Device; use radicle::prelude::*; @@ -31,14 +32,14 @@ pub fn run( fn edit_root<G>( mut patch: patch::PatchMut<'_, '_, Repository, cob::cache::StoreWriter>, - title: String, + title: Title, description: String, signer: &Device<G>, ) -> anyhow::Result<()> where G: crypto::signature::Signer<crypto::Signature>, { - let title = if title != patch.title() { + let title = if title.as_ref() != patch.title() { Some(title) } else { None @@ -74,7 +75,7 @@ where fn edit_revision<G>( mut patch: patch::PatchMut<'_, '_, Repository, cob::cache::StoreWriter>, revision: patch::RevisionId, - mut title: String, + title: Title, description: String, signer: &Device<G>, ) -> anyhow::Result<()> @@ -82,15 +83,16 @@ where G: crypto::signature::Signer<crypto::Signature>, { let embeds = patch.embeds().to_owned(); - let description = if description.is_empty() { - title + let mut message = title.to_string(); + let message = if description.is_empty() { + message } else { - title.push('\n'); - title.push_str(&description); - title + message.push('\n'); + message.push_str(&description); + message }; patch.transaction("Edit revision", signer, |tx| { - tx.edit_revision(revision, description, embeds)?; + tx.edit_revision(revision, message, embeds)?; Ok(()) })?; Ok(()) diff --git a/crates/radicle-cli/src/commands/publish.rs b/crates/radicle-cli/src/commands/publish.rs index 146f80b9..c6244811 100644 --- a/crates/radicle-cli/src/commands/publish.rs +++ b/crates/radicle-cli/src/commands/publish.rs @@ -2,6 +2,7 @@ use std::ffi::OsString; use anyhow::{anyhow, Context as _}; +use radicle::cob; use radicle::identity::{Identity, Visibility}; use radicle::node::Handle as _; use radicle::prelude::RepoId; @@ -105,7 +106,15 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { doc.visibility = Visibility::Public; })?; - identity.update("Publish repository", "", &doc, &signer)?; + // SAFETY: the `Title` here is guaranteed to be nonempty and does not + // contain `\n` or `\r`. + #[allow(clippy::unwrap_used)] + identity.update( + cob::Title::new("Publish repository").unwrap(), + "", + &doc, + &signer, + )?; repo.sign_refs(&signer)?; repo.set_identity_head()?; let validations = repo.validate()?; diff --git a/crates/radicle-cli/src/terminal/issue.rs b/crates/radicle-cli/src/terminal/issue.rs index 2c019335..7480e32a 100644 --- a/crates/radicle-cli/src/terminal/issue.rs +++ b/crates/radicle-cli/src/terminal/issue.rs @@ -32,9 +32,9 @@ pub enum Format { } pub fn get_title_description( - title: Option<String>, + title: Option<cob::Title>, description: Option<String>, -) -> io::Result<Option<(String, String)>> { +) -> io::Result<Option<(cob::Title, String)>> { term::patch::Message::edit_title_description(title, description, OPEN_MSG) } diff --git a/crates/radicle-cli/src/terminal/patch.rs b/crates/radicle-cli/src/terminal/patch.rs index b8956a1e..5136bc6b 100644 --- a/crates/radicle-cli/src/terminal/patch.rs +++ b/crates/radicle-cli/src/terminal/patch.rs @@ -10,6 +10,7 @@ use thiserror::Error; use radicle::cob; use radicle::cob::patch; +use radicle::cob::Title; use radicle::git; use radicle::patch::{Patch, PatchId}; use radicle::prelude::Profile; @@ -72,14 +73,14 @@ impl Message { /// Open the editor with the given title and description (if any). /// Returns the edited title and description, or nothing if it couldn't be parsed. pub fn edit_title_description( - title: Option<String>, + title: Option<cob::Title>, description: Option<String>, help: &str, - ) -> std::io::Result<Option<(String, String)>> { + ) -> std::io::Result<Option<(Title, String)>> { let mut placeholder = String::new(); if let Some(title) = title { - placeholder.push_str(title.trim()); + placeholder.push_str(title.as_ref()); placeholder.push('\n'); } if let Some(description) = description { @@ -94,12 +95,12 @@ impl Message { Some((x, y)) => (x, y), None => (output.as_str(), ""), }; - let (title, description) = (title.trim(), description.trim()); - if title.is_empty() | title.contains('\n') { + let Ok(title) = Title::new(title) else { return Ok(None); - } - Ok(Some((title.to_owned(), description.to_owned()))) + }; + + Ok(Some((title, description.trim().to_owned()))) } pub fn append(&mut self, arg: &str) { @@ -220,20 +221,19 @@ pub fn get_create_message( repo: &git::raw::Repository, base: &git::Oid, head: &git::Oid, -) -> Result<(String, String), Error> { +) -> Result<(Title, String), Error> { let display_msg = create_display_message(repo, base, head)?; let message = message.get(&display_msg)?; let (title, description) = message.split_once('\n').unwrap_or((&message, "")); let (title, description) = (title.trim().to_string(), description.trim().to_string()); - if title.is_empty() { - return Err(io::Error::new( + let title = Title::new(title.as_str()).map_err(|err| { + io::Error::new( io::ErrorKind::InvalidInput, - "a patch title must be provided", + format!("invalid patch title: {err}"), ) - .into()); - } + })?; Ok((title, description)) } @@ -249,7 +249,7 @@ fn edit_display_message(title: &str, description: &str) -> String { pub fn get_edit_message( patch_message: term::patch::Message, patch: &cob::patch::Patch, -) -> io::Result<(String, String)> { +) -> io::Result<(Title, String)> { let display_msg = edit_display_message(patch.title(), patch.description()); let patch_message = patch_message.get(&display_msg)?; let patch_message = patch_message.replace(PATCH_MSG.trim(), ""); // Delete help message. @@ -259,12 +259,12 @@ pub fn get_edit_message( .unwrap_or((&patch_message, "")); let (title, description) = (title.trim().to_string(), description.trim().to_string()); - if title.is_empty() { - return Err(io::Error::new( + let title = Title::new(title.as_str()).map_err(|err| { + io::Error::new( io::ErrorKind::InvalidInput, - "a patch title must be provided", - )); - } + format!("invalid patch title: {err}"), + ) + })?; Ok((title, description)) } diff --git a/crates/radicle-cli/tests/commands.rs b/crates/radicle-cli/tests/commands.rs index 9c7ae275..0d5e53ae 100644 --- a/crates/radicle-cli/tests/commands.rs +++ b/crates/radicle-cli/tests/commands.rs @@ -2,6 +2,7 @@ use std::path::Path; use std::str::FromStr; use std::{net, thread, time}; +use radicle::cob; use radicle::git; use radicle::node; use radicle::node::address::Store as _; @@ -1654,7 +1655,7 @@ fn test_cob_replication() { let mut bob_cache = radicle::cob::cache::InMemory::default(); let issue = bob_issues .create( - "Something's fishy", + cob::Title::new("Something's fishy").unwrap(), "I don't know what it is", &[], &[], @@ -1705,7 +1706,7 @@ fn test_cob_deletion() { let mut alice_issues = radicle::cob::issue::Cache::no_cache(&alice_repo).unwrap(); let issue = alice_issues .create( - "Something's fishy", + cob::Title::new("Something's fishy").unwrap(), "I don't know what it is", &[], &[], diff --git a/crates/radicle-node/src/test/node.rs b/crates/radicle-node/src/test/node.rs index 5de098d3..25b6c5e4 100644 --- a/crates/radicle-node/src/test/node.rs +++ b/crates/radicle-node/src/test/node.rs @@ -344,7 +344,7 @@ impl<G: Signer<Signature> + cyphernet::Ecdh> NodeHandle<G> { } /// Create an [`issue::Issue`] in the `NodeHandle`'s storage. - pub fn issue(&self, rid: RepoId, title: &str, desc: &str) -> cob::ObjectId { + pub fn issue(&self, rid: RepoId, title: cob::Title, desc: &str) -> cob::ObjectId { let repo = self.storage.repository(rid).unwrap(); let mut issues = issue::Cache::no_cache(&repo).unwrap(); *issues diff --git a/crates/radicle-node/src/tests.rs b/crates/radicle-node/src/tests.rs index faf4aa4f..75d1c4f3 100644 --- a/crates/radicle-node/src/tests.rs +++ b/crates/radicle-node/src/tests.rs @@ -9,6 +9,7 @@ use std::sync::LazyLock; use std::time; use crossbeam_channel as chan; +use radicle::cob; use radicle::identity::Visibility; use radicle::node::address::Store as _; use radicle::node::device::Device; @@ -1013,7 +1014,14 @@ fn test_refs_announcement_offline() { let old_refs = RefsAt::new(&repo, alice.id).unwrap(); let mut issues = radicle::issue::Cache::no_cache(&repo).unwrap(); issues - .create("Issue while offline!", "", &[], &[], [], alice.signer()) + .create( + cob::Title::new("Issue while offline!").unwrap(), + "", + &[], + &[], + [], + alice.signer(), + ) .unwrap(); let new_refs = RefsAt::new(&repo, alice.id).unwrap(); assert_ne!(old_refs, new_refs); diff --git a/crates/radicle-node/src/tests/e2e.rs b/crates/radicle-node/src/tests/e2e.rs index 0f42260d..c82e656d 100644 --- a/crates/radicle-node/src/tests/e2e.rs +++ b/crates/radicle-node/src/tests/e2e.rs @@ -1,5 +1,6 @@ use std::{collections::HashSet, thread, time}; +use radicle::cob::Title; use test_log::test; use radicle::node::device::Device; @@ -399,7 +400,7 @@ fn test_dont_fetch_owned_refs() { log::debug!(target: "test", "Fetch complete with {}", bob.id); - alice.issue(acme, "Don't fetch self", "Use ^"); + alice.issue(acme, Title::new("Don't fetch self").unwrap(), "Use ^"); let result = alice.handle.fetch(acme, bob.id, DEFAULT_TIMEOUT).unwrap(); assert!(result.is_success()) } @@ -478,7 +479,11 @@ fn test_missing_remote() { log::debug!(target: "test", "Fetch complete with {}", bob.id); rad::fork_remote(acme, &alice.id, &carol, &bob.storage).unwrap(); - alice.issue(acme, "Missing Remote", "Fixing the missing remote issue"); + alice.issue( + acme, + Title::new("Missing Remote").unwrap(), + "Fixing the missing remote issue", + ); let result = bob.handle.fetch(acme, alice.id, DEFAULT_TIMEOUT).unwrap(); assert!(result.is_success()); log::debug!(target: "test", "Fetch complete with {}", bob.id); @@ -504,7 +509,7 @@ fn test_fetch_preserve_owned_refs() { log::debug!(target: "test", "Fetch complete with {}", bob.id); - alice.issue(acme, "Bug", "Bugs, bugs, bugs"); + alice.issue(acme, Title::new("Bug").unwrap(), "Bugs, bugs, bugs"); let before = alice .storage @@ -906,7 +911,7 @@ fn test_non_fastforward_sigrefs() { // Bob updates his refs. bob.issue( rid, - "Updated Sigrefs", + Title::new("Updated Sigrefs").unwrap(), "Updated sigrefs are harshing my vibes", ); // Alice fetches from Bob. @@ -999,7 +1004,7 @@ fn test_outdated_sigrefs() { let issue_id = eve.issue( rid, - "Outdated Sigrefs", + Title::new("Outdated Sigrefs").unwrap(), "Outdated sigrefs are harshing my vibes", ); let repo = eve.storage.repository(rid).unwrap(); @@ -1093,7 +1098,7 @@ fn test_outdated_delegate_sigrefs() { alice.issue( rid, - "Outdated Sigrefs", + Title::new("Outdated Sigrefs").unwrap(), "Outdated sigrefs are harshing my vibes", ); let repo = alice.storage.repository(rid).unwrap(); @@ -1150,7 +1155,11 @@ fn missing_default_branch() { // Fetching from still works despite not having // `refs/heads/master`, but has `rad/sigrefs`. - bob.issue(rid, "Hello, Acme", "Popping in to say hello"); + bob.issue( + rid, + Title::new("Hello, Acme").unwrap(), + "Popping in to say hello", + ); alice.handle.fetch(rid, bob.id, DEFAULT_TIMEOUT).unwrap(); { @@ -1244,7 +1253,9 @@ fn missing_delegate_default_branch() { doc.delegate(bob.signer.public_key().into()); }) .unwrap(); - let rev = identity.update("Add Bob", "", &doc, &alice.signer).unwrap(); + let rev = identity + .update(Title::new("Add Bob").unwrap(), "", &doc, &alice.signer) + .unwrap(); repo.set_identity_head_to(rev).unwrap(); let new = repo.identity_doc().unwrap().doc; @@ -1261,7 +1272,7 @@ fn missing_delegate_default_branch() { // Create an issue to ensure there are new refs to fetch let issue = bob.issue( rid, - "Delegate Issue", + Title::new("Delegate Issue").unwrap(), "Further investigation into delegates", ); let assert_bobs_issue_exists = |repo: &Repository| { @@ -1349,7 +1360,7 @@ fn test_background_foreground_fetch() { // the new refs eve.issue( rid, - "Outdated Sigrefs", + Title::new("Outdated Sigrefs").unwrap(), "Outdated sigrefs are harshing my vibes", ); let repo = eve.storage.repository(rid).unwrap(); @@ -1363,7 +1374,7 @@ fn test_background_foreground_fetch() { .unwrap(); bob.issue( rid, - "Concurrent fetches", + Title::new("Concurrent fetches").unwrap(), "Concurrent fetches are harshing my vibes", ); bob.handle.announce_refs(rid).unwrap(); @@ -1414,7 +1425,7 @@ fn test_catchup_on_refs_announcements() { bob.has_repository(&acme); log::debug!(target: "test", "Bob creating his issue.."); - bob.issue(acme, "Bob's issue", "[..]"); + bob.issue(acme, Title::new("Bob's issue").unwrap(), "[..]"); bob.handle.announce_refs(acme).unwrap(); log::debug!(target: "test", "Waiting for seed to fetch Bob's refs from Bob.."); diff --git a/crates/radicle-remote-helper/src/push.rs b/crates/radicle-remote-helper/src/push.rs index 53a0e278..56b3678e 100644 --- a/crates/radicle-remote-helper/src/push.rs +++ b/crates/radicle-remote-helper/src/push.rs @@ -500,7 +500,7 @@ where let patch = if opts.draft { patches.draft( - &title, + title, &description, patch::MergeTarget::default(), base, @@ -510,7 +510,7 @@ where ) } else { patches.create( - &title, + title, &description, patch::MergeTarget::default(), base, diff --git a/crates/radicle/src/cob/common.rs b/crates/radicle/src/cob/common.rs index 90924886..639b8e40 100644 --- a/crates/radicle/src/cob/common.rs +++ b/crates/radicle/src/cob/common.rs @@ -7,6 +7,7 @@ use std::str::FromStr; use base64::prelude::{Engine, BASE64_STANDARD}; use localtime::LocalTime; use serde::{Deserialize, Serialize}; +use thiserror::Error; use crate::git::Oid; use crate::prelude::{Did, PublicKey}; @@ -42,6 +43,64 @@ impl Deref for Timestamp { } } +#[derive(Error, Debug)] +pub enum TitleError { + #[error("empty title")] + EmptyTitle, + #[error("invalid characters in title")] + InvalidTitle, +} + +/// A `Title` is used for messages that are included in collaborative objects, +/// such as patches, issues, and identity changes. +/// +/// A `Title`: +/// - Must not be empty +/// - Must not contain `\n` or `\r` characters +/// - Will be trimmed of any preceding or following whitespace +#[derive(Display, Deserialize, Serialize, PartialEq, Eq, Clone, Debug)] +#[display(inner)] +pub struct Title(String); + +impl Title { + /// # Errors + /// + /// [`TitleError::EmptyTitle`]: the provided `title` was empty + /// [`TitleError::InvalidTitle`]: the provided `title` contained invalid + /// characters + pub fn new(title: &str) -> Result<Self, TitleError> { + if title.contains('\n') || title.contains('\r') { + Err(TitleError::InvalidTitle) + } else if title.is_empty() { + Err(TitleError::EmptyTitle) + } else { + Ok(Self(title.trim().to_string())) + } + } +} + +impl AsRef<str> for Title { + fn as_ref(&self) -> &str { + &self.0 + } +} + +impl FromStr for Title { + type Err = TitleError; + + fn from_str(s: &str) -> Result<Self, Self::Err> { + Self::new(s) + } +} + +impl TryFrom<String> for Title { + type Error = TitleError; + + fn try_from(value: String) -> Result<Self, Self::Error> { + Self::new(&value) + } +} + /// Author. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord)] pub struct Author { diff --git a/crates/radicle/src/cob/identity.rs b/crates/radicle/src/cob/identity.rs index fad01e2e..938936fd 100644 --- a/crates/radicle/src/cob/identity.rs +++ b/crates/radicle/src/cob/identity.rs @@ -55,7 +55,7 @@ pub enum Action { #[serde(rename = "revision")] Revision { /// Short summary of changes. - title: String, + title: cob::Title, /// Longer comment on proposed changes. #[serde(default, skip_serializing_if = "String::is_empty")] description: String, @@ -71,7 +71,7 @@ pub enum Action { /// The revision to edit. revision: RevisionId, /// Short summary of changes. - title: String, + title: cob::Title, /// Longer comment on proposed changes. #[serde(default, skip_serializing_if = "String::is_empty")] description: String, @@ -217,7 +217,18 @@ impl Identity { "Initialize identity", &mut store, signer, - |tx, repo| tx.revision("Initial revision", "", doc, None, repo, signer), + |tx, repo| { + tx.revision( + // SAFETY: "Initial revision" is a valid title + #[allow(clippy::unwrap_used)] + cob::Title::new("Initial revision").unwrap(), + "", + doc, + None, + repo, + signer, + ) + }, )?; Ok(IdentityMut { @@ -690,7 +701,7 @@ pub struct Revision { /// Identity document blob at this revision. pub blob: Oid, /// Title of the proposal. - pub title: String, + pub title: cob::Title, /// State of the revision. pub state: State, /// Description of the proposal. @@ -759,7 +770,7 @@ impl Revision { impl Revision { fn new( id: RevisionId, - title: String, + title: cob::Title, description: String, author: Author, blob: Oid, @@ -841,12 +852,12 @@ impl<R: ReadRepository> store::Transaction<Identity, R> { pub fn edit( &mut self, revision: RevisionId, - title: impl ToString, + title: cob::Title, description: impl ToString, ) -> Result<(), store::Error> { self.push(Action::RevisionEdit { revision, - title: title.to_string(), + title, description: description.to_string(), }) } @@ -859,7 +870,7 @@ impl<R: ReadRepository> store::Transaction<Identity, R> { impl<R: WriteRepository> store::Transaction<Identity, R> { pub fn revision<G: crypto::signature::Signer<crypto::Signature>>( &mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, doc: &Doc, parent: Option<RevisionId>, @@ -877,7 +888,7 @@ impl<R: WriteRepository> store::Transaction<Identity, R> { // Revision metadata. self.push(Action::Revision { - title: title.to_string(), + title, description: description.to_string(), blob, parent, @@ -939,7 +950,7 @@ where /// If the signer is the only delegate, the revision is accepted automatically. pub fn update<G>( &mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, doc: &Doc, signer: &Device<G>, @@ -987,7 +998,7 @@ where pub fn edit<G>( &mut self, revision: RevisionId, - title: String, + title: cob::Title, description: String, signer: &Device<G>, ) -> Result<EntryId, Error> @@ -1043,7 +1054,7 @@ mod lookup { mod test { use qcheck_macros::quickcheck; - use crate::cob; + use crate::cob::{self, Title}; use crate::crypto::PublicKey; use crate::identity::did::Did; use crate::identity::doc::PayloadId; @@ -1075,7 +1086,7 @@ mod test { let signer = &node.signer; let mut identity = Identity::load_mut(&*repo).unwrap(); let mut doc = identity.doc().clone().edit(); - let title = "Identity update"; + let title = Title::new("Identity update").unwrap(); let description = ""; let r0 = identity.current; @@ -1083,7 +1094,12 @@ mod test { assert!(identity.current().is_accepted()); // Using an identical document to the current one fails. identity - .update(title, description, &doc.clone().verified().unwrap(), signer) + .update( + title.clone(), + description, + &doc.clone().verified().unwrap(), + signer, + ) .unwrap_err(); assert_eq!(identity.current, r0); @@ -1096,7 +1112,12 @@ mod test { doc.delegate(bob.public_key().into()); // The update should go through now. let r1 = identity - .update(title, description, &doc.clone().verified().unwrap(), signer) + .update( + title.clone(), + description, + &doc.clone().verified().unwrap(), + signer, + ) .unwrap(); assert!(identity.revision(&r1).unwrap().is_accepted()); assert_eq!(identity.current, r1); @@ -1105,7 +1126,12 @@ mod test { // signs it. doc.visibility = Visibility::private([]); let r2 = identity - .update(title, description, &doc.clone().verified().unwrap(), signer) + .update( + title.clone(), + description, + &doc.clone().verified().unwrap(), + signer, + ) .unwrap(); // R1 is still the head. assert_eq!(identity.current, r1); @@ -1136,20 +1162,24 @@ mod test { let signer = &node.signer; let mut identity = Identity::load_mut(&*repo).unwrap(); let mut doc = identity.doc().clone().edit(); - let title = "Identity update"; let description = ""; // Let's add another delegate. doc.delegate(bob.public_key().into()); let r1 = identity - .update(title, description, &doc.clone().verified().unwrap(), signer) + .update( + cob::Title::new("Identity update").unwrap(), + description, + &doc.clone().verified().unwrap(), + signer, + ) .unwrap(); assert_eq!(identity.current, r1); doc.visibility = Visibility::private([]); let r2 = identity .update( - "Make private", + cob::Title::new("Make private").unwrap(), description, &doc.clone().verified().unwrap(), &node.signer, @@ -1165,7 +1195,7 @@ mod test { doc.delegate(eve.public_key().into()); let r3 = identity .update( - "Add Eve", + cob::Title::new("Add Eve").unwrap(), description, &doc.clone().verified().unwrap(), &node.signer, @@ -1177,7 +1207,7 @@ mod test { doc.visibility = Visibility::Public; let r3 = identity .update( - "Make public", + cob::Title::new("Make public").unwrap(), description, &doc.verified().unwrap(), &node.signer, @@ -1207,7 +1237,7 @@ mod test { alice_doc.delegate(bob.signer.public_key().into()); let a1 = alice_identity .update( - "Add Bob", + cob::Title::new("Add Bob").unwrap(), "", &alice_doc.clone().verified().unwrap(), &alice.signer, @@ -1224,7 +1254,7 @@ mod test { alice_doc.visibility = Visibility::private([]); let a2 = alice_identity .update( - "Change visibility", + cob::Title::new("Change visibility").unwrap(), "", &alice_doc.clone().clone().verified().unwrap(), &alice.signer, @@ -1233,7 +1263,7 @@ mod test { // Bob makes the same change without knowing Alice already did. let b1 = bob_identity .update( - "Make private", + cob::Title::new("Make private").unwrap(), "", &alice_doc.verified().unwrap(), &bob.signer, @@ -1275,7 +1305,7 @@ mod test { let a0 = alice_identity.root; let a1 = alice_identity .update( - "Add Bob", + cob::Title::new("Add Bob").unwrap(), "Eh.", &alice_doc.clone().clone().verified().unwrap(), &alice.signer, @@ -1285,7 +1315,7 @@ mod test { alice_doc.visibility = Visibility::private([eve.signer.public_key().into()]); let a2 = alice_identity .update( - "Change visibility", + cob::Title::new("Change visibility").unwrap(), "Eh.", &alice_doc.verified().unwrap(), &alice.signer, @@ -1325,7 +1355,7 @@ mod test { let a0 = alice_identity.root; let a1 = alice_identity // Change description to change traversal order. .update( - "Add Bob and Eve", + cob::Title::new("Add Bob and Eve").unwrap(), "Eh#!", &alice_doc.clone().verified().unwrap(), &alice.signer, @@ -1335,7 +1365,7 @@ mod test { alice_doc.rescind(&eve.signer.public_key().into()).unwrap(); let a2 = alice_identity .update( - "Remove Eve", + cob::Title::new("Remove Eve").unwrap(), "", &alice_doc.verified().unwrap(), &alice.signer, @@ -1358,7 +1388,7 @@ mod test { let e1 = cob::git::stable::with_advanced_timestamp(|| { eve_identity .update( - "Change visibility", + cob::Title::new("Change visibility").unwrap(), "", &eve_doc.verified().unwrap(), &eve.signer, @@ -1401,7 +1431,7 @@ mod test { let a0 = alice_identity.root; let a1 = alice_identity .update( - "Add Bob and Eve", + cob::Title::new("Add Bob and Eve").unwrap(), "Eh!#", &alice_doc.clone().verified().unwrap(), &alice.signer, @@ -1411,7 +1441,7 @@ mod test { alice_doc.visibility = Visibility::private([]); let a2 = alice_identity .update( - "Change visibility", + cob::Title::new("Change visibility").unwrap(), "", &alice_doc.verified().unwrap(), &alice.signer, @@ -1436,7 +1466,7 @@ mod test { eve_doc.visibility = Visibility::private([eve.signer.public_key().into()]); let e2 = eve_identity .update( - "Change visibility", + cob::Title::new("Change visibility").unwrap(), "", &eve_doc.verified().unwrap(), &eve.signer, @@ -1487,7 +1517,7 @@ mod test { let a0 = alice_identity.root; let a1 = alice_identity .update( - "Add Bob and Eve", + cob::Title::new("Add Bob and Eve").unwrap(), "", &alice_doc.verified().unwrap(), &alice.signer, @@ -1513,7 +1543,7 @@ mod test { bob_doc.visibility = Visibility::private([]); let b1 = bob_identity .update( - "Change visibility #1", + cob::Title::new("Change visibility #1").unwrap(), "", &bob_doc.verified().unwrap(), &bob.signer, @@ -1528,7 +1558,7 @@ mod test { eve_doc.visibility = Visibility::private([]); let e1 = eve_identity .update( - "Change visibility #2", + cob::Title::new("Change visibility #2").unwrap(), "Woops", &eve_doc.verified().unwrap(), &eve.signer, @@ -1578,7 +1608,7 @@ mod test { doc.payload.insert(PayloadId::project(), prj.clone().into()); identity .update( - "Update description", + cob::Title::new("Update description").unwrap(), "", &doc.clone().verified().unwrap(), &alice, @@ -1589,7 +1619,12 @@ mod test { doc.delegate(bob.public_key().into()); doc.threshold = 2; identity - .update("Add bob", "", &doc.clone().verified().unwrap(), &alice) + .update( + cob::Title::new("Add bob").unwrap(), + "", + &doc.clone().verified().unwrap(), + &alice, + ) .unwrap(); // Add Eve as a delegate. @@ -1597,7 +1632,12 @@ mod test { // Update with both Bob and Alice's signature. let revision = identity - .update("Add eve", "", &doc.clone().verified().unwrap(), &alice) + .update( + cob::Title::new("Add eve").unwrap(), + "", + &doc.clone().verified().unwrap(), + &alice, + ) .unwrap(); identity.accept(&revision, &bob).unwrap(); @@ -1608,7 +1648,7 @@ mod test { let revision = identity .update( - "Update description again", + cob::Title::new("Update description again").unwrap(), "Bob's repository", &doc.verified().unwrap(), &bob, diff --git a/crates/radicle/src/cob/issue.rs b/crates/radicle/src/cob/issue.rs index 21a50f10..f771301c 100644 --- a/crates/radicle/src/cob/issue.rs +++ b/crates/radicle/src/cob/issue.rs @@ -12,9 +12,9 @@ use crate::cob; use crate::cob::common::{Author, Authorization, Label, Reaction, Timestamp, Uri}; use crate::cob::store::Transaction; use crate::cob::store::{Cob, CobAction}; -use crate::cob::thread; use crate::cob::thread::{Comment, CommentId, Thread}; use crate::cob::{op, store, ActorId, Embed, EntryId, ObjectId, TypeName}; +use crate::cob::{thread, TitleError}; use crate::identity::doc::DocError; use crate::node::device::Device; use crate::node::NodeId; @@ -53,6 +53,8 @@ pub enum Error { Thread(#[from] thread::Error), #[error("store: {0}")] Store(#[from] store::Error), + #[error("invalid issue title due to: {0}")] + TitleError(#[from] TitleError), /// Action not authorized. #[error("{0} not authorized to apply {1:?}")] NotAuthorized(ActorId, Action), @@ -424,10 +426,7 @@ impl Issue { self.assignees = BTreeSet::from_iter(assignees); } Action::Edit { title } => { - if title.contains('\n') || title.contains('\r') { - return Err(Error::InvalidTitle(title)); - } - self.title = title; + self.title = title.to_string(); } Action::Lifecycle { state } => { self.state = state; @@ -511,10 +510,8 @@ impl<R: ReadRepository> store::Transaction<Issue, R> { } /// Set the issue title. - pub fn edit(&mut self, title: impl ToString) -> Result<(), store::Error> { - self.push(Action::Edit { - title: title.to_string(), - }) + pub fn edit(&mut self, title: cob::Title) -> Result<(), store::Error> { + self.push(Action::Edit { title }) } /// Redact a comment. @@ -631,7 +628,7 @@ where } /// Set the issue title. - pub fn edit<G>(&mut self, title: impl ToString, signer: &Device<G>) -> Result<EntryId, Error> + pub fn edit<G>(&mut self, title: cob::Title, signer: &Device<G>) -> Result<EntryId, Error> where G: crypto::signature::Signer<crypto::Signature>, { @@ -821,7 +818,7 @@ where /// Create a new issue. pub fn create<'g, G, C>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, labels: &[Label], assignees: &[Did], @@ -922,7 +919,7 @@ pub enum Action { /// Edit issue title. #[serde(rename = "edit")] - Edit { title: String }, + Edit { title: cob::Title }, /// Transition to a different state. #[serde(rename = "lifecycle")] @@ -998,7 +995,7 @@ mod test { let mut eve_issues = Cache::no_cache(&*t.eve.repo).unwrap(); let mut issue_alice = issues_alice .create( - "Alice Issue", + cob::Title::new("Alice Issue").unwrap(), "Alice's comment", &[], &[], @@ -1087,7 +1084,7 @@ mod test { let assignee_two = Did::from(arbitrary::gen::<ActorId>(1)); let issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[assignee], @@ -1126,7 +1123,7 @@ mod test { let assignee_two = Did::from(arbitrary::gen::<ActorId>(1)); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[assignee, assignee_two], @@ -1156,7 +1153,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let created = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1182,7 +1179,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1225,7 +1222,7 @@ mod test { let assignee_two = Did::from(arbitrary::gen::<ActorId>(1)); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[assignee, assignee_two], @@ -1251,7 +1248,7 @@ mod test { let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1260,7 +1257,9 @@ mod test { ) .unwrap(); - issue.edit("Sorry typo", &node.signer).unwrap(); + issue + .edit(cob::Title::new("Sorry typo").unwrap(), &node.signer) + .unwrap(); let id = issue.id; let issue = issues.get(&id).unwrap().unwrap(); @@ -1275,7 +1274,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1301,7 +1300,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1331,7 +1330,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1389,7 +1388,7 @@ mod test { let wontfix_label = Label::new("wontfix").unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[ux_label.clone()], &[], @@ -1424,7 +1423,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1464,7 +1463,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1511,13 +1510,34 @@ mod test { let test::setup::NodeWithRepo { node, repo, .. } = test::setup::NodeWithRepo::default(); let mut issues = Cache::no_cache(&*repo).unwrap(); issues - .create("First", "Blah", &[], &[], [], &node.signer) + .create( + cob::Title::new("First").unwrap(), + "Blah", + &[], + &[], + [], + &node.signer, + ) .unwrap(); issues - .create("Second", "Blah", &[], &[], [], &node.signer) + .create( + cob::Title::new("Second").unwrap(), + "Blah", + &[], + &[], + [], + &node.signer, + ) .unwrap(); issues - .create("Third", "Blah", &[], &[], [], &node.signer) + .create( + cob::Title::new("Third").unwrap(), + "Blah", + &[], + &[], + [], + &node.signer, + ) .unwrap(); let issues = issues @@ -1540,7 +1560,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let created = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.\nYah yah yah", &[], &[], @@ -1583,7 +1603,7 @@ mod test { }; let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1642,7 +1662,7 @@ mod test { }; let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1674,7 +1694,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1708,7 +1728,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1735,7 +1755,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], @@ -1766,7 +1786,7 @@ mod test { let mut issues = Cache::no_cache(&*repo).unwrap(); let mut issue = issues .create( - "My first issue", + cob::Title::new("My first issue").unwrap(), "Blah blah blah.", &[], &[], diff --git a/crates/radicle/src/cob/issue/cache.rs b/crates/radicle/src/cob/issue/cache.rs index 7bd035b7..b986f246 100644 --- a/crates/radicle/src/cob/issue/cache.rs +++ b/crates/radicle/src/cob/issue/cache.rs @@ -100,7 +100,7 @@ impl<'a, R, C> Cache<super::Issues<'a, R>, C> { /// main storage, and writing the update to the `cache`. pub fn create<'g, G>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, labels: &[Label], assignees: &[Did], diff --git a/crates/radicle/src/cob/patch.rs b/crates/radicle/src/cob/patch.rs index 8c2daeca..64422550 100644 --- a/crates/radicle/src/cob/patch.rs +++ b/crates/radicle/src/cob/patch.rs @@ -175,7 +175,10 @@ pub enum Action { // Actions on patch. // #[serde(rename = "edit")] - Edit { title: String, target: MergeTarget }, + Edit { + title: cob::Title, + target: MergeTarget, + }, #[serde(rename = "label")] Label { labels: BTreeSet<Label> }, #[serde(rename = "lifecycle")] @@ -422,7 +425,7 @@ impl MergeTarget { #[serde(rename_all = "camelCase")] pub struct Patch { /// Title of the patch. - pub(super) title: String, + pub(super) title: cob::Title, /// Patch author. pub(super) author: Author, /// Current state of the patch. @@ -455,7 +458,11 @@ pub struct Patch { impl Patch { /// Construct a new patch object from a revision. - pub fn new(title: String, target: MergeTarget, (id, revision): (RevisionId, Revision)) -> Self { + pub fn new( + title: cob::Title, + target: MergeTarget, + (id, revision): (RevisionId, Revision), + ) -> Self { Self { title, author: revision.author.clone(), @@ -472,7 +479,7 @@ impl Patch { /// Title of the patch. pub fn title(&self) -> &str { - self.title.as_str() + self.title.as_ref() } /// Current state of the patch. @@ -1742,11 +1749,8 @@ impl Review { } impl<R: ReadRepository> store::Transaction<Patch, R> { - pub fn edit(&mut self, title: impl ToString, target: MergeTarget) -> Result<(), store::Error> { - self.push(Action::Edit { - title: title.to_string(), - target, - }) + pub fn edit(&mut self, title: cob::Title, target: MergeTarget) -> Result<(), store::Error> { + self.push(Action::Edit { title, target }) } pub fn edit_revision( @@ -2095,7 +2099,7 @@ where /// Edit patch metadata. pub fn edit<G, S>( &mut self, - title: String, + title: cob::Title, target: MergeTarget, signer: &Device<G>, ) -> Result<EntryId, Error> @@ -2678,7 +2682,7 @@ where /// Open a new patch. pub fn create<'g, C, G>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, target: MergeTarget, base: impl Into<git::Oid>, @@ -2707,7 +2711,7 @@ where /// Draft a patch. This patch will be created in a [`State::Draft`] state. pub fn draft<'g, C, G>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, target: MergeTarget, base: impl Into<git::Oid>, @@ -2755,7 +2759,7 @@ where /// Create a patch. This is an internal function used by `create` and `draft`. fn _create<'g, C, G>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, target: MergeTarget, base: impl Into<git::Oid>, @@ -3060,7 +3064,7 @@ mod test { let target = MergeTarget::Delegates; let patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", target, branch.base, @@ -3100,7 +3104,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3133,7 +3137,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3164,7 +3168,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3216,7 +3220,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3263,7 +3267,7 @@ mod test { resolves: Default::default(), }, Action::Edit { - title: String::from("My patch"), + title: cob::Title::new("My patch").unwrap(), target: MergeTarget::Delegates, }, ]); @@ -3314,7 +3318,7 @@ mod test { resolves: Default::default(), }, Action::Edit { - title: String::from("Some patch"), + title: cob::Title::new("Some patch").unwrap(), target: MergeTarget::Delegates, }, ], @@ -3374,7 +3378,7 @@ mod test { resolves: Default::default(), }, Action::Edit { - title: String::from("My patch"), + title: cob::Title::new("My patch").unwrap(), target: MergeTarget::Delegates, }, ]); @@ -3404,7 +3408,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3449,7 +3453,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3480,7 +3484,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3531,7 +3535,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3578,7 +3582,7 @@ mod test { let mut patches = Cache::no_cache(&*alice.repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3633,7 +3637,7 @@ mod test { }; let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, @@ -3682,7 +3686,7 @@ mod test { let mut patches = Cache::no_cache(&*repo).unwrap(); let mut patch = patches .create( - "My first patch", + cob::Title::new("My first patch").unwrap(), "Blah blah blah.", MergeTarget::Delegates, branch.base, diff --git a/crates/radicle/src/cob/patch/cache.rs b/crates/radicle/src/cob/patch/cache.rs index ea10869d..4e8ff004 100644 --- a/crates/radicle/src/cob/patch/cache.rs +++ b/crates/radicle/src/cob/patch/cache.rs @@ -110,7 +110,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> { /// main storage, and writing the update to the `cache`. pub fn create<'g, G>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, target: MergeTarget, base: impl Into<git::Oid>, @@ -140,7 +140,7 @@ impl<'a, R, C> Cache<super::Patches<'a, R>, C> { /// to the `cache`. pub fn draft<'g, G>( &'g mut self, - title: impl ToString, + title: cob::Title, description: impl ToString, target: MergeTarget, base: impl Into<git::Oid>, @@ -710,9 +710,8 @@ mod tests { use radicle_cob::ObjectId; use crate::cob::cache::{Store, Update, Write}; - use crate::cob::migrate; use crate::cob::thread::{Comment, Thread}; - use crate::cob::Author; + use crate::cob::{migrate, Author, Title}; use crate::patch::{ ByRevision, MergeTarget, Patch, PatchCounts, PatchId, Revision, RevisionId, State, Status, }; @@ -767,13 +766,21 @@ mod tests { let mut cache = memory(repo); assert!(cache.is_empty().unwrap()); - let patch = Patch::new("Patch #1".to_string(), MergeTarget::Delegates, revision()); + let patch = Patch::new( + Title::new("Patch #1").unwrap(), + MergeTarget::Delegates, + revision(), + ); let id = ObjectId::from_str("47799cbab2eca047b6520b9fce805da42b49ecab").unwrap(); cache.update(&cache.rid(), &id, &patch).unwrap(); let patch = Patch { state: State::Archived, - ..Patch::new("Patch #2".to_string(), MergeTarget::Delegates, revision()) + ..Patch::new( + Title::new("Patch #2").unwrap(), + MergeTarget::Delegates, + revision(), + ) }; let id = ObjectId::from_str("ae981ded6ed2ed2cdba34c8603714782667f18a3").unwrap(); cache.update(&cache.rid(), &id, &patch).unwrap(); @@ -803,7 +810,11 @@ mod tests { .collect::<BTreeSet<PatchId>>(); for id in open_ids.iter() { - let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision()); + let patch = Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ); cache .update(&cache.rid(), &PatchId::from(*id), &patch) .unwrap(); @@ -812,7 +823,11 @@ mod tests { for id in draft_ids.iter() { let patch = Patch { state: State::Draft, - ..Patch::new(id.to_string(), MergeTarget::Delegates, revision()) + ..Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ) }; cache .update(&cache.rid(), &PatchId::from(*id), &patch) @@ -822,7 +837,11 @@ mod tests { for id in archived_ids.iter() { let patch = Patch { state: State::Archived, - ..Patch::new(id.to_string(), MergeTarget::Delegates, revision()) + ..Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ) }; cache .update(&cache.rid(), &PatchId::from(*id), &patch) @@ -835,7 +854,11 @@ mod tests { revision: arbitrary::oid().into(), commit: arbitrary::oid(), }, - ..Patch::new(id.to_string(), MergeTarget::Delegates, revision()) + ..Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ) }; cache .update(&cache.rid(), &PatchId::from(*id), &patch) @@ -869,7 +892,11 @@ mod tests { let mut patches = Vec::with_capacity(ids.len()); for id in ids.iter() { - let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision()); + let patch = Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ); cache .update(&cache.rid(), &PatchId::from(*id), &patch) .unwrap(); @@ -898,7 +925,7 @@ mod tests { .next() .expect("at least one revision should have been created"); let mut patch = Patch::new( - patch_id.to_string(), + Title::new(&patch_id.to_string()).unwrap(), MergeTarget::Delegates, (*rev_id, rev.clone()), ); @@ -937,7 +964,11 @@ mod tests { let mut patches = Vec::with_capacity(ids.len()); for id in ids.iter() { - let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision()); + let patch = Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ); cache .update(&cache.rid(), &PatchId::from(*id), &patch) .unwrap(); @@ -964,7 +995,11 @@ mod tests { let mut patches = Vec::with_capacity(ids.len()); for id in ids.iter() { - let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision()); + let patch = Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ); cache .update(&cache.rid(), &PatchId::from(*id), &patch) .unwrap(); @@ -990,7 +1025,11 @@ mod tests { .collect::<BTreeSet<PatchId>>(); for id in ids.iter() { - let patch = Patch::new(id.to_string(), MergeTarget::Delegates, revision()); + let patch = Patch::new( + Title::new(&id.to_string()).unwrap(), + MergeTarget::Delegates, + revision(), + ); cache .update(&cache.rid(), &PatchId::from(*id), &patch) .unwrap(); diff --git a/crates/radicle/src/cob/test.rs b/crates/radicle/src/cob/test.rs index c2fb701c..30fb221f 100644 --- a/crates/radicle/src/cob/test.rs +++ b/crates/radicle/src/cob/test.rs @@ -7,9 +7,9 @@ use radicle_crypto::ssh::ExtendedSignature; use serde::{Deserialize, Serialize}; use crate::cob::op::Op; -use crate::cob::patch; use crate::cob::patch::Patch; use crate::cob::store::encoding; +use crate::cob::{patch, Title}; use crate::cob::{Entry, History, Manifest, Timestamp, Version}; use crate::crypto::Signer; use crate::git; @@ -224,7 +224,7 @@ impl<G: Signer> Actor<G> { /// Create a patch. pub fn patch<R: ReadRepository>( &mut self, - title: impl ToString, + title: Title, description: impl ToString, base: git::Oid, oid: git::Oid, @@ -239,7 +239,7 @@ impl<G: Signer> Actor<G> { resolves: Default::default(), }, patch::Action::Edit { - title: title.to_string(), + title, target: patch::MergeTarget::default(), }, ]), diff --git a/crates/radicle/src/storage/refs.rs b/crates/radicle/src/storage/refs.rs index 895a7c84..750f8f71 100644 --- a/crates/radicle/src/storage/refs.rs +++ b/crates/radicle/src/storage/refs.rs @@ -477,7 +477,7 @@ mod tests { use super::*; use crate::assert_matches; - use crate::{cob::identity::Identity, rad, test::fixtures, Storage}; + use crate::{cob::identity::Identity, cob::Title, rad, test::fixtures, Storage}; #[quickcheck] fn prop_canonical_roundtrip(refs: Refs) { @@ -554,10 +554,10 @@ mod tests { let mut london_ident = Identity::load_mut(&london).unwrap(); paris_ident - .update("Add Bob", "", &paris_doc, &alice) + .update(Title::new("Add Bob").unwrap(), "", &paris_doc, &alice) .unwrap(); london_ident - .update("Add Bob", "", &london_doc, &alice) + .update(Title::new("Add Bob").unwrap(), "", &london_doc, &alice) .unwrap(); }