Skip to content

Commit

Permalink
Merge #503
Browse files Browse the repository at this point in the history
503: Un-breaking-change Uuid::new_v4 r=KodrAus a=CAD97

<!--
    If this PR is a breaking change, ensure that you are opening it against 
    the `breaking` branch.  If the pull request is incomplete, prepend the Title with WIP: 
-->

**I'm submitting a(n)** refactor

# Description

#447 changed from using `rand::thread_rng` to using `getrandom` in `Uuid::new_v4`. This also changed the return type from `Uuid` to `Result<Uuid, getrandom::Error>`. This PR reverts the signature to the simpler `new_v4() -> Uuid`.

# Motivation

This signature is much simpler to use, and avoids a breaking change. `getrandom` is _highly_ unlikely to fail, and previously we used `thread_rng` here, which also panics if it fails to initialize from the OS entropy source. If `getrandom` fails, it is highly unlikely that any program creating v4 UUIDs has any reasonable recovery other than to abort, as the system is in a broken state.

If users absolutely need to recover in this situation, they can call `getrandom` first themselves to make sure that their system is working, or generate the bytes themselves and create the UUID from those bytes.

Additionally, actually wrapping `getrandom::Error` in `uuid::Error` comes with its own fun set of problems, described in #502.

# Tests

N/A

# Related Issue(s)

#475: this undoes the breaking change to `Uuid::new_v4`, thus making the requested publish a patch update

Closes #502: alternate approach to the same TODO

Co-authored-by: CAD97 <cad97@cad97.com>
  • Loading branch information
bors[bot] and CAD97 committed Dec 21, 2020
2 parents 379e3e4 + 23e235b commit 97e6bf5
Showing 1 changed file with 13 additions and 12 deletions.
25 changes: 13 additions & 12 deletions src/v4.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ impl Uuid {
/// Creates a random UUID.
///
/// This uses the [`getrandom`] crate to utilise the operating system's RNG
/// as the source of random numbers. If you'd like to use a custom generator,
/// don't use this method: generate random bytes using your custom generator
/// and pass them to the [`uuid::Builder::from_bytes`][from_bytes] function
/// instead.
/// as the source of random numbers. If you'd like to use a custom
/// generator, don't use this method: generate random bytes using your
/// custom generator and pass them to the
/// [`uuid::Builder::from_bytes`][from_bytes] function instead.
///
/// Note that usage of this method requires the `v4` feature of this crate
/// to be enabled.
Expand All @@ -24,16 +24,17 @@ impl Uuid {
///
/// [`getrandom`]: https://crates.io/crates/getrandom
/// [from_bytes]: struct.Builder.html#method.from_bytes
// TODO: change signature to support uuid's Error.
pub fn new_v4() -> Result<Uuid, getrandom::Error> {
pub fn new_v4() -> Uuid {
let mut bytes = [0u8; 16];
getrandom::getrandom(&mut bytes)?;
getrandom::getrandom(&mut bytes).unwrap_or_else(|err| {
// NB: getrandom::Error has no source; this is adequate display
panic!("could not retreive random bytes for uuid: {}", err)
});

let uuid = crate::Builder::from_bytes(bytes)
crate::Builder::from_bytes(bytes)
.set_variant(Variant::RFC4122)
.set_version(Version::Random)
.build();
Ok(uuid)
.build()
}
}

Expand All @@ -43,15 +44,15 @@ mod tests {

#[test]
fn test_new() {
let uuid = Uuid::new_v4().unwrap();
let uuid = Uuid::new_v4();

assert_eq!(uuid.get_version(), Some(Version::Random));
assert_eq!(uuid.get_variant(), Some(Variant::RFC4122));
}

#[test]
fn test_get_version() {
let uuid = Uuid::new_v4().unwrap();
let uuid = Uuid::new_v4();

assert_eq!(uuid.get_version(), Some(Version::Random));
assert_eq!(uuid.get_version_num(), 4)
Expand Down

0 comments on commit 97e6bf5

Please sign in to comment.