Previously it was possible to create an issue with an empty description
(i.e. an empty "root" comment), and also to change it later to a
non-empty string, however it was not possible to clear it again.
I was greeted by `rad patch redact` with
called `Result::unwrap()` on an `Err` value:
Error { code: None, message: Some("failed to convert") }
stack backtrace:
[…]
3: core::result::Result<T,E>::unwrap
at …/rust-1.88.0/lib/rustlib/src/rust/library/core/src/result.rs:1137:23
4: sqlite::cursor::Row::read
at …/index.crates.io-1949cf8c6b5b557f/sqlite-0.32.0/src/cursor.rs:136:9
5: radicle::cob::patch::cache::query::find_by_revision
at ./crates/radicle/src/cob/patch/cache.rs:624:65
6: <… as radicle::cob::patch::cache::Patches>::find_by_revision
at ./crates/radicle/src/cob/patch/cache.rs:553:9
7: radicle_cli::commands::rad_patch::redact::run
at ./crates/radicle-cli/src/commands/patch/redact.rs:23:9
8: radicle_cli::commands::rad_patch::run
at ./crates/radicle-cli/src/commands/patch.rs:1026:13
[…]
It turns out that `sqlite::cursor::Row::read` is the panicky version of
`sqlite::cursor::Row::try_read` (which returns a `Result`).
While it is somewhat rare that SQLite reads fail, it is not unheard of.
In `radicle-cli` it might not be critical, but also `radicle-protocol`
and `radicle-fetch` are affected, and they could potentially panic a
`radicle-node` process.
Use `try_read` instead, and propagate down the error handling.
The stream tests did not take into account that the tips of previous COB entries
need to be inserted so that a Git history can be formed.
This was surfaced by the added regression test.
If the tests are run without a global Git config, then the tests will error with
no name or email set for the commits.
Instead, use the test fixture setup to ensure there is a name and email for
running the tests.
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 <fintan.halpenny@gmail.com>
Add type aliases to specialise the `Stream` type for each of `Patch`, `Issue`,
and `Identity`.
An `init` constructor is also added for each to make it easier to construct each
stream.
Introduce a new `stream` module to `radicle::cob`. The purpose of this module is
to provide an API for iterating over a COB's operations, given a range of
commits.
The `CobStream` trait provides the generic API for implementing and using, while
`Stream` provides a concrete implementation using the `git2` crate.
The approach is to use a `git2::Revwalk` for walking over each commit in the
range, then loading the `Op` for that commit – skipping any commits that are
unrelated to that COB type, in the case of associated COBs.
The `Stream` is tested using some properties of the API.
The next steps are to introduce specialised types for `Patch` and `Issue` to
ensure that it works for those.
Adds an `Op::load` helper method for loading an `Entry`, given its `Oid` and the
`store` it is stored in.
This can be useful for downstream consumers to inspect operations on COBs given
a single `Oid`, without having to load the entire object and/or graph.
It can also be useful when looking at implementing COB stream primitives.
Add a helper method `Op::manifest_of`, to allow the caller to inspect the
`Manifest` of the entry. This can be used to avoid attempting to call `Op:load`
with a manifest that would not match the expected `Action`s of the `Op`.
This patch adds an implementation of the Iterator::size_hint() trait
function so that the compiler can optimize allocations on the collecting
side of the iterator better.
From the trait function docs:
> size_hint() is primarily intended to be used for optimizations such
> as reserving space for the elements of the iterator, but must not be
> trusted to e.g., omit bounds checks in unsafe code. An incorrect
> implementation of size_hint() should not lead to memory safety
> violations.
Source: https://doc.rust-lang.org/stable/std/iter/trait.Iterator.html#method.size_hint
And:
> Vec may use any or none of the following strategies, depending on the
> supplied iterator:
>
> * preallocate based on Iterator::size_hint() and panic if the number
> of items is outside the provided lower/upper bounds
Source: https://doc.rust-lang.org/stable/std/iter/trait.FromIterator.html
Signed-off-by: Matthias Beyer <mail@beyermatthias.de>
After upgrading from Rust 1.85 to 1.88 in 586eefc, clippy started
warning about large error types. Box action to make
`radicle::cob::patch::Error` lighter.
Downstream clients require access to the data for a `ReviewEdit` – for example,
the desktop client builds a representation of `Action`s for TypeScript
generation.
To allow these kinds of use cases, methods are added to `ReviewEdit` to allow
access to its underlying data.
The `Review::summary` was limited in behaviour, since it was only an
`Option<String>`.
The field is now changed to `NonEmpty<Edit>` so that it can enable an edit
history, as well as being able to supply embeds as part of the text.
`Reactions` are also introduced to `Review` and, necessarily, introducing an
`Action::ReviewReact`.
Note that some machinery was added under `actions` and `encoding` to enable
backwards-compatibility.
For `actions`, a new `ReviewEdit` is introduced to keep
track of the different versions of editing actions – supporting the old actions
while also introducing the new variant.
For `encoding`, the machinery exploits the fact that an `Option<String>` can be
converted into a `NonEmpty<Edit>` given that an `ActorId` and `Timestamp` are
available, and that `None` can be converted into an empty string. The struct
`patch::Review` now deserializes via this `encoding::review::Review` type and,
importantly, preserves backwards-compatibility.
Defelo
Lorenz Leutgeb
Fintan Halpenny
Sebastian Martinez
Matthias Beyer
Erik Kundt