make core rust functions non-erroring

Author: carderne

README.md

@@ -11,7 +11,7 @@ UPID is based on [ULID](https://github.com/ulid/spec) but with some modification
 The core idea is that a **meaningful prefix** is specified that is stored in a 128-bit UUID-shaped slot.
 Thus a UPID is **human-readable** (like a Stripe ID), but still efficient to store, sort and index.
 
-UPID allows a prefix of up to **4 characters** (will be right-padded if shorter than 4), includes a non-wrapping timestamp with about 300 millisecond precision, and 64 bits of entropy.
+UPID allows a prefix of up to **4 characters** (will be right-padded if shorter than 4), includes a non-wrapping timestamp with about 250 millisecond precision, and 64 bits of entropy.
 
 This is a UPID in Python:
 ```python
@@ -81,7 +81,7 @@ Key changes relative to ULID:
 ### Collision
 Relative to ULID, the time precision is reduced from 48 to 40 bits (keeping the most significant bits, so oveflow still won't occur until 10889 AD), and the randomness reduced from 80 to 64 bits.
 
-The timestamp precision at 40 bits is around 300 milliseconds. In order to have a 50% probability of collision with 64 bits of randomness, you would need to generate around **4 billion items per 100 millisecond window**.
+The timestamp precision at 40 bits is around 250 milliseconds. In order to have a 50% probability of collision with 64 bits of randomness, you would need to generate around **4 billion items per 250 millisecond window**.
 
 ## Python implementation
 This aims to be maximally simple to convey the core working of the spec.
@@ -105,11 +105,19 @@ upid("user")
 ```
 
 #### Development
+Code and tests are in the [py/](./py/) directory. Using [Rye](https://rye.astral.sh/) for development (installation instructions at the link).
+
 ```bash
+# can be run from the repo root
 rye sync
 rye run all  # or fmt/lint/check/test
 ```
 
+If you just want to have a look around, pip should also work:
+```bash
+pip install -e .
+```
+
 ## Rust implementation
 The current Rust implementation is based on [dylanhart/ulid-rs](https://github.com/dylanhart/ulid-rs), but using the same lookup base32 lookup method as the Python implementation.
 
@@ -125,28 +133,32 @@ Upid::new("user");
 ```
 
 #### Development
+Code and tests are in the [upid_rs/](./upid_rs/) directory.
+
 ```bash
-cargo check  # or fmt/clippy/test/run
+cd upid_rs
+cargo check  # or fmt/clippy/build/test/run
 ```
 
 ## Postgres extension
 There is also a Postgres extension built on the Rust implementation, using [pgrx](https://github.com/pgcentralfoundation/pgrx) and based on the very similar extension [pksunkara/pgx_ulid](https://github.com/pksunkara/pgx_ulid).
 
 #### Installation
-You will need to install pgrx and follow its installation instructions.
+You can try out the Docker image [carderne/postgres-upid:16](https://hub.docker.com/r/carderne/postgres-upid):
+```bash
+docker run -e POSTGRES_HOST_AUTH_METHOD=trust -p 5432:5432 carderne/postgres-upid:16
+```
+
+If you want to install it into another Postgres, you'll install pgrx and follow its [installation instructions](https://github.com/pgcentralfoundation/pgrx/blob/develop/cargo-pgrx/README.md).
 Something like this:
 ```bash
 cargo install --locked cargo-pgrx
 pgrx init
 cd upid_pg
 pgrx install
-pgrx run
 ```
 
-Alternatively, you can try out the Docker image `[carderne/postgres-upid:16](https://hub.docker.com/r/carderne/postgres-upid):
-```bash
-docker run -e POSTGRES_HOST_AUTH_METHOD=trust -p 5432:5432 carderne/postgres-upid:16
-```
+Installable binaries will come soon.
 
 #### Usage
 ```sql
@@ -162,6 +174,14 @@ SELECT * FROM users;
 ```
 
 #### Development
+Code and tests are in the [upid_pg/](./upid_pg/) directory.
+
 ```bash
-cargo pgrx test
+cd upid_pg
+cargo check  # or fmt/clippy
+
+# must test/run/install with pgrx
+# this will compile it into a Postgres installation
+# and run the tests there, or drop you into a psql prompt
+cargo pgrx test  # or run/install
 ```

py/tests/test_core.py

@@ -54,3 +54,16 @@ def test_datetime_roundtrip() -> None:
     got = a.datetime
     diff = want - got
     assert diff.total_seconds() * consts.MS_PER_SEC < TS_EPS
+
+
+def test_invalid_prefix() -> None:
+    # Invalid characters just become 'zzzz'
+    want = "zzzz"
+
+    # even if too long
+    got = UPID.from_prefix("[0#/]]1,").prefix
+    assert got == want
+
+    # or too short
+    got = UPID.from_prefix("[0").prefix
+    assert got == want

py/upid/core.py

@@ -21,6 +21,8 @@ class UPID:
     """
     The `UPID` contains a 20-bit prefix, 40-bit timestamp and 68 bits of randomness.
 
+    The prefix should only contain lower-case latin alphabet characters.
+
     It is usually created using the `upid(prefix: str)` helper function:
 
         upid("user")  # UPID(user_3accvpp5_guht4dts56je5w)

upid_pg/src/lib.rs

@@ -81,7 +81,7 @@ impl FromDatum for upid {
 
 #[pg_extern]
 fn gen_upid(prefix: &str) -> upid {
-    upid(InnerUpid::new(prefix).unwrap().0)
+    upid(InnerUpid::new(prefix).0)
 }
 
 #[pg_extern(immutable, parallel_safe)]

upid_rs/src/lib.rs

@@ -27,29 +27,35 @@ pub struct Upid(pub u128);
 
 impl Upid {
     /// Creates a new Upid with the provided prefix and current time (UTC)
+    ///
+    /// The prefix should only contain lower-case latin alphabet characters.
     /// # Example
     /// ```rust
     /// use upid::Upid;
     ///
     /// let my_upid = Upid::new("user");
     /// ```
-    pub fn new(prefix: &str) -> Result<Upid, DecodeError> {
+    pub fn new(prefix: &str) -> Upid {
         Upid::from_prefix(prefix)
     }
 
     /// Creates a Upid with the provided prefix and current time (UTC)
+    ///
+    /// The prefix should only contain lower-case latin alphabet characters.
     /// # Example
     /// ```rust
     /// use upid::Upid;
     ///
     /// let my_upid = Upid::from_prefix("user");
     /// ```
-    pub fn from_prefix(prefix: &str) -> Result<Upid, DecodeError> {
+    pub fn from_prefix(prefix: &str) -> Upid {
         Upid::from_prefix_and_datetime(prefix, now())
     }
 
     /// Creates a new Upid with the given prefix and datetime
     ///
+    /// The prefix should only contain lower-case latin alphabet characters.
+    ///
     /// This will take the maximum of the `[SystemTime]` argument and `[SystemTime::UNIX_EPOCH]`
     /// as earlier times are not valid for a Upid timestamp
     ///
@@ -60,10 +66,7 @@ impl Upid {
     ///
     /// let upid = Upid::from_prefix_and_datetime("user", SystemTime::now());
     /// ```
-    pub fn from_prefix_and_datetime(
-        prefix: &str,
-        datetime: SystemTime,
-    ) -> Result<Upid, DecodeError> {
+    pub fn from_prefix_and_datetime(prefix: &str, datetime: SystemTime) -> Upid {
         let milliseconds = datetime
             .duration_since(SystemTime::UNIX_EPOCH)
             .unwrap_or(Duration::ZERO)
@@ -73,17 +76,16 @@ impl Upid {
 
     /// Creates a new Upid with the given prefix and timestamp in millisecons
     ///
+    /// The prefix should only contain lower-case latin alphabet characters.
+    ///
     /// # Example
     /// ```rust
     /// use upid::Upid;
     ///
     /// let ms: u128 = 1720568902000;
     /// let upid = Upid::from_prefix_and_milliseconds("user", ms);
     /// ```
-    pub fn from_prefix_and_milliseconds(
-        prefix: &str,
-        milliseconds: u128,
-    ) -> Result<Upid, DecodeError> {
+    pub fn from_prefix_and_milliseconds(prefix: &str, milliseconds: u128) -> Upid {
         // cut off the 8 lsb drops precision to 256 ms
         // future version could play with this differently
         // eg drop 4 bits on each side
@@ -97,15 +99,21 @@ impl Upid {
         let prefix = format!("{:z<4}", prefix);
         let prefix: String = prefix.chars().take(4).collect();
         let prefix = format!("{}{}", prefix, VERSION);
-        let p = b32::decode_prefix(prefix.as_bytes())?;
+
+        // decode_prefix Errors if the last character is past 'j' in the b32 alphabet
+        // and we control that with the VERSION variable
+        // If the prefix has characters from outside the alphabet, they will be wrapped into 'z's
+        // And we have ensured above that it is exactly 5 characters long
+        let p = b32::decode_prefix(prefix.as_bytes())
+            .expect("decode_prefix failed with version character overflow");
 
         let res = (time_bits << 88)
             | (random << 24)
             | ((p[0] as u128) << 16)
             | ((p[1] as u128) << 8)
             | p[2] as u128;
 
-        Ok(Upid(res))
+        Upid(res)
     }
 
     /// Gets the datetime of when this Upid was created accurate to around 300ms
@@ -116,7 +124,7 @@ impl Upid {
     /// use upid::Upid;
     ///
     /// let dt = SystemTime::now();
-    /// let upid = Upid::from_prefix_and_datetime("user", dt).unwrap();
+    /// let upid = Upid::from_prefix_and_datetime("user", dt);
     ///
     /// assert!(dt + Duration::from_millis(300) >= upid.datetime());
     /// ```
@@ -134,7 +142,6 @@ impl Upid {
     /// let text = "user_aaccvpp5guht4dts56je5a";
     /// let result = Upid::from_string(text);
     ///
-    /// assert!(result.is_ok());
     /// assert_eq!(&result.unwrap().to_string(), text);
     /// ```
     pub fn from_string(encoded: &str) -> Result<Upid, DecodeError> {
@@ -151,7 +158,7 @@ impl Upid {
     /// use upid::Upid;
     ///
     /// let prefix = "user";
-    /// let upid = Upid::from_prefix(prefix).unwrap();
+    /// let upid = Upid::from_prefix(prefix);
     ///
     /// assert_eq!(upid.prefix(), prefix);
     /// ```
@@ -168,7 +175,7 @@ impl Upid {
     /// use upid::Upid;
     ///
     /// let ms: u128 = 1720568902000;
-    /// let upid = Upid::from_prefix_and_milliseconds("user", ms).unwrap();
+    /// let upid = Upid::from_prefix_and_milliseconds("user", ms);
     ///
     /// assert!(ms - u128::from(upid.milliseconds()) < 256);
     /// ```
@@ -221,7 +228,7 @@ impl Upid {
 
 impl Default for Upid {
     fn default() -> Self {
-        Upid::new("").unwrap()
+        Upid::new("")
     }
 }
 
@@ -281,7 +288,7 @@ mod tests {
 
     #[test]
     fn test_dynamic() {
-        let upid = Upid::new("user").unwrap();
+        let upid = Upid::new("user");
         let encoded = upid.to_string();
         let upid2 = Upid::from_string(&encoded).expect("failed to deserialize");
         assert_eq!(upid, upid2);
@@ -290,9 +297,8 @@ mod tests {
     #[test]
     fn test_order() {
         let dt = SystemTime::now();
-        let upid1 = Upid::from_prefix_and_datetime("user", dt).unwrap();
-        let upid2 =
-            Upid::from_prefix_and_datetime("user", dt + Duration::from_millis(300)).unwrap();
+        let upid1 = Upid::from_prefix_and_datetime("user", dt);
+        let upid2 = Upid::from_prefix_and_datetime("user", dt + Duration::from_millis(300));
         assert!(upid1 < upid2);
     }
 
@@ -303,7 +309,7 @@ mod tests {
             .duration_since(SystemTime::UNIX_EPOCH)
             .unwrap()
             .as_millis();
-        let upid = Upid::from_prefix_and_milliseconds("user", want).unwrap();
+        let upid = Upid::from_prefix_and_milliseconds("user", want);
         let got = u128::from(upid.milliseconds());
 
         assert!(want - got < EPS);
@@ -312,9 +318,23 @@ mod tests {
     #[test]
     fn test_datetime() {
         let dt = SystemTime::now();
-        let upid = Upid::from_prefix_and_datetime("user", dt).unwrap();
+        let upid = Upid::from_prefix_and_datetime("user", dt);
 
         assert!(upid.datetime() <= dt);
         assert!(upid.datetime() + Duration::from_millis(300) >= dt);
     }
+
+    #[test]
+    fn test_invalid_prefix() {
+        // Invalid characters just become 'zzzz'
+        let want = "zzzz";
+
+        // even if too long
+        let got = Upid::from_prefix("[0#/]]1,").prefix();
+        assert_eq!(got, want);
+
+        // or too short
+        let got = Upid::from_prefix("[0").prefix();
+        assert_eq!(got, want);
+    }
 }

upid_rs/src/main.rs

@@ -8,5 +8,5 @@ fn main() {
         Some(value) => value,
         None => &"".to_string(),
     };
-    println!("{}", Upid::from_prefix(prefix).unwrap().to_string());
+    println!("{}", Upid::from_prefix(prefix).to_string());
 }