Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

All feature combinations compile #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@
name = "ulid"
version = "1.0.0"
authors = ["dylanhart <[email protected]>"]
edition = "2018"
edition = "2021"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this strictly for resolver=2?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is for the question mark syntax in serde?/std and uuid?/std. I suspect it is needed for that, otherwise I might have changed it because there shouldn't be anything that supports the optional-dependency-feature syntax but doesn't support 2021.

rust-version = "1.60"

license = "MIT"
readme = "README.md"
Expand All @@ -14,12 +15,12 @@ repository = "https://github.com/dylanhart/ulid-rs"

[features]
default = ["std"]
std = ["rand"]
std = ["rand", "serde?/std", "uuid?/std"]

[dependencies]
serde = { version = "1.0", features = ["derive"], optional = true }
serde = { version = "1.0", features = ["derive"], optional = true, default-features = false }
rand = { version = "0.8", optional = true }
uuid = { version = "1.1", optional = true }
uuid = { version = "1.1", optional = true, default-features = false }

[dev-dependencies]
bencher = "0.1"
Expand All @@ -34,3 +35,4 @@ members = ["cli"]

[package.metadata.docs.rs]
all-features = true
rustdoc-args = ["--cfg", "docsrs"]
3 changes: 2 additions & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
name = "ulid-cli"
version = "0.3.1"
authors = ["dylanhart <[email protected]>", "Kan-Ru Chen <[email protected]>"]
rust-version = "1.60"

license = "MIT"
readme = "../README.md"
Expand All @@ -11,7 +12,7 @@ keywords = ["ulid", "uuid", "sortable", "identifier"]

repository = "https://github.com/dylanhart/ulid-rs"

edition = "2018"
edition = "2021"

[dependencies]
structopt = "0.2"
Expand Down
3 changes: 3 additions & 0 deletions src/base32.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ pub enum EncodeError {
}

#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl std::error::Error for EncodeError {}

impl fmt::Display for EncodeError {
Expand Down Expand Up @@ -73,6 +74,7 @@ pub fn encode_to(mut value: u128, buffer: &mut [u8]) -> Result<usize, EncodeErro
}

#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn encode(value: u128) -> String {
let mut buffer: [u8; ULID_LEN] = [0; ULID_LEN];

Expand All @@ -91,6 +93,7 @@ pub enum DecodeError {
}

#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl std::error::Error for DecodeError {}

impl fmt::Display for DecodeError {
Expand Down
2 changes: 2 additions & 0 deletions src/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::fmt;
use crate::Ulid;

/// A Ulid generator that provides monotonically increasing Ulids
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub struct Generator {
previous: Ulid,
}
Expand Down Expand Up @@ -152,6 +153,7 @@ impl Default for Generator {
}

/// Error while trying to generate a monotonic increment in the same millisecond
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
#[derive(Debug, PartialEq, Eq, Hash, Copy, Clone)]
pub enum MonotonicError {
/// Would overflow into the next millisecond
Expand Down
4 changes: 4 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
//!
//! ```
#![cfg_attr(not(feature = "std"), no_std)]
#![cfg_attr(docsrs, feature(doc_cfg))]

#[doc = include_str!("../README.md")]
#[cfg(all(doctest, feature = "std"))]
Expand All @@ -39,6 +40,7 @@ mod base32;
#[cfg(feature = "std")]
mod generator;
#[cfg(feature = "serde")]
#[cfg_attr(docsrs, doc(cfg(feature = "serde")))]
pub mod serde;
#[cfg(feature = "std")]
mod time;
Expand Down Expand Up @@ -206,6 +208,7 @@ impl Ulid {
/// ```
#[allow(clippy::inherent_to_string_shadow_display)] // Significantly faster than Display::to_string
#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn to_string(&self) -> String {
base32::encode(self.0)
}
Expand Down Expand Up @@ -281,6 +284,7 @@ impl Default for Ulid {
}

#[cfg(feature = "std")]
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl From<Ulid> for String {
fn from(ulid: Ulid) -> String {
ulid.to_string()
Expand Down
70 changes: 60 additions & 10 deletions src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,63 @@ impl Serialize for Ulid {
}
}

struct UlidVisitor(&'static str);
impl<'de> serde::de::Visitor<'de> for UlidVisitor {
type Value = Ulid;

fn expecting(&self, formatter: &mut core::fmt::Formatter) -> core::fmt::Result {
formatter.write_str(self.0)
}

fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
#[cfg(feature = "uuid")]
if matches!(value.len(), 32 | 36 | 38 | 45) {
return match uuid::Uuid::try_parse(value) {
Ok(a) => Ok(Ulid::from(a)),
Err(e) => Err(serde::de::Error::custom(e)),
};
}
Ulid::from_string(value).map_err(serde::de::Error::custom)
}

fn visit_bytes<E>(self, v: &[u8]) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
// allow either the 16 bytes as is, or a wrongly typed str
match v.len() {
16 => {
let ptr = v.as_ptr() as *const [u8; 16];
Ok(Ulid::from_bytes(*unsafe { &*ptr }))
}
crate::ULID_LEN => Ulid::from_string(unsafe { core::str::from_utf8_unchecked(v) })

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the bytes in v assumed to be valid utf8?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be somewhat of an over-the-top optimization, but from_string(v: &str) only interacts with the string variable using v.len() and v.as_bytes() and doesn't actually use characters or char boundaries in any way. A better solution that still doesn't unnecessarily validate the string twice would have been to change base32::decode(_: &str) to base32::decode(_: &[u8]) and call that directly instead.

Copy link

@TroyKomodo TroyKomodo Oct 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, i see, so because of line 79 deserializer.deserialize_str(UlidVisitor("an ulid string or value")) it forces the uft-8 to be valid. Understood thanks.

Out of curiosity what conditions need to be met for this code path to be hit?

Also would it be benificial to comment a safety block above here encompassing the reason this is safe.

I think it should be noted that you would need to be careful because if you add support to deserialize from bytes directly using serde this immediately becomes unsafe. Perhaps it might be worth while to validate the bytes anyways.

.map_err(serde::de::Error::custom),
#[cfg(feature = "uuid")]
32 | 36 | 38 | 45 => match uuid::Uuid::try_parse_ascii(v) {
Ok(a) => Ok(Ulid::from(a)),
Err(e) => Err(serde::de::Error::custom(e)),
},
len => Err(E::invalid_length(len, &self.0)),
}
}

fn visit_u128<E>(self, v: u128) -> Result<Self::Value, E>
where
E: serde::de::Error,
{
Ok(Ulid(v))
}
}

impl<'de> Deserialize<'de> for Ulid {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
let deserialized_str = String::deserialize(deserializer)?;
Self::from_string(&deserialized_str).map_err(serde::de::Error::custom)
deserializer.deserialize_str(UlidVisitor("an ulid string or value"))
}
}

Expand All @@ -50,7 +100,7 @@ impl<'de> Deserialize<'de> for Ulid {
/// ```
pub mod ulid_as_u128 {
use crate::Ulid;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use serde::{Deserializer, Serialize, Serializer};

/// Serializes a ULID as a u128 type.
pub fn serialize<S>(value: &Ulid, serializer: S) -> Result<S::Ok, S::Error>
Expand All @@ -65,8 +115,7 @@ pub mod ulid_as_u128 {
where
D: Deserializer<'de>,
{
let deserialized_u128 = u128::deserialize(deserializer)?;
Ok(Ulid(deserialized_u128))
deserializer.deserialize_u128(super::UlidVisitor("an ulid value as u128"))
}
}

Expand All @@ -89,9 +138,10 @@ pub mod ulid_as_u128 {
/// }
/// ```
#[cfg(all(feature = "uuid", feature = "serde"))]
#[cfg_attr(docsrs, doc(cfg(all(feature = "uuid", feature = "serde"))))]
pub mod ulid_as_uuid {
use crate::Ulid;
use serde::{Deserialize, Deserializer, Serialize, Serializer};
use serde::{Deserializer, Serializer};
use uuid::Uuid;

/// Converts the ULID to a UUID and serializes it as a string.
Expand All @@ -100,16 +150,16 @@ pub mod ulid_as_uuid {
S: Serializer,
{
let uuid: Uuid = (*value).into();
uuid.to_string().serialize(serializer)
let mut buffer = uuid::Uuid::encode_buffer();
let form = uuid.as_hyphenated().encode_lower(&mut buffer);
serializer.serialize_str(form)
}

/// Deserializes a ULID from a string containing a UUID.
pub fn deserialize<'de, D>(deserializer: D) -> Result<Ulid, D::Error>
where
D: Deserializer<'de>,
{
let de_string = String::deserialize(deserializer)?;
let de_uuid = Uuid::parse_str(&de_string).map_err(serde::de::Error::custom)?;
Ok(Ulid::from(de_uuid))
deserializer.deserialize_str(super::UlidVisitor("an uuid string"))
}
}
8 changes: 8 additions & 0 deletions src/time.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
use crate::{bitmask, Ulid};
use std::time::{Duration, SystemTime};

/// The standard library can be used to get the current time.
/// The `std` feature (which is enabled by default) will expose some extra functions to make life easier.
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
impl Ulid {
/// Creates a new Ulid with the current time (UTC)
///
Expand All @@ -10,6 +13,7 @@ impl Ulid {
///
/// let my_ulid = Ulid::new();
/// ```
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn new() -> Ulid {
Ulid::from_datetime(SystemTime::now())
}
Expand All @@ -24,6 +28,7 @@ impl Ulid {
/// let mut rng = StdRng::from_entropy();
/// let ulid = Ulid::with_source(&mut rng);
/// ```
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn with_source<R: rand::Rng>(source: &mut R) -> Ulid {
Ulid::from_datetime_with_source(SystemTime::now(), source)
}
Expand All @@ -42,6 +47,7 @@ impl Ulid {
///
/// let ulid = Ulid::from_datetime(SystemTime::now());
/// ```
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn from_datetime(datetime: SystemTime) -> Ulid {
Ulid::from_datetime_with_source(datetime, &mut rand::thread_rng())
}
Expand All @@ -60,6 +66,7 @@ impl Ulid {
/// let mut rng = StdRng::from_entropy();
/// let ulid = Ulid::from_datetime_with_source(SystemTime::now(), &mut rng);
/// ```
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn from_datetime_with_source<R>(datetime: SystemTime, source: &mut R) -> Ulid
where
R: rand::Rng + ?Sized,
Expand Down Expand Up @@ -90,6 +97,7 @@ impl Ulid {
/// && dt - Duration::from_millis(1) <= ulid.datetime()
/// );
/// ```
#[cfg_attr(docsrs, doc(cfg(feature = "std")))]
pub fn datetime(&self) -> SystemTime {
let stamp = self.timestamp_ms();
SystemTime::UNIX_EPOCH + Duration::from_millis(stamp)
Expand Down
19 changes: 17 additions & 2 deletions src/uuid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@
use crate::Ulid;
use uuid::Uuid;

#[cfg_attr(docsrs, doc(cfg(feature = "uuid")))]
impl From<Uuid> for Ulid {
fn from(uuid: Uuid) -> Self {
Ulid(uuid.as_u128())
}
}

#[cfg_attr(docsrs, doc(cfg(feature = "uuid")))]
impl From<Ulid> for Uuid {
fn from(ulid: Ulid) -> Self {
Uuid::from_u128(ulid.0)
Expand All @@ -21,7 +23,14 @@ mod test {

#[test]
fn uuid_cycle() {
#[cfg(feature = "std")]
let ulid = Ulid::new();
#[cfg(not(feature = "std"))]
let ulid = Ulid::from_parts(
0x0000_1020_3040_5060_u64,
0x0000_0000_0000_0102_0304_0506_0708_090A_u128,
);

let uuid: Uuid = ulid.into();
let ulid2: Ulid = uuid.into();

Expand All @@ -32,11 +41,17 @@ mod test {
fn uuid_str_cycle() {
let uuid_txt = "771a3bce-02e9-4428-a68e-b1e7e82b7f9f";
let ulid_txt = "3Q38XWW0Q98GMAD3NHWZM2PZWZ";
let mut buf = uuid::Uuid::encode_buffer();
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.

Do you find this method useful? It might be nice to add something similar to Ulid.


let ulid: Ulid = Uuid::parse_str(uuid_txt).unwrap().into();
assert_eq!(ulid.to_string(), ulid_txt);
let ulid_str = ulid.to_str(&mut buf).unwrap();
assert_eq!(ulid_str, ulid_txt);

#[cfg(feature = "std")]
assert_eq!(ulid.to_string().as_str(), ulid_txt);

let uuid: Uuid = ulid.into();
assert_eq!(uuid.to_string(), uuid_txt);
let uuid_str = uuid.hyphenated().encode_lower(&mut buf);
assert_eq!(uuid_str, uuid_txt);
}
}