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

feat(json_types): implement FromStr for ValidAccountId #391

Merged
merged 19 commits into from
May 11, 2021
Merged
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
76 changes: 55 additions & 21 deletions near-sdk/src/json_types/account.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,28 @@
use borsh::{BorshDeserialize, BorshSerialize};
use serde::Serialize;
use std::convert::{TryFrom, TryInto};
use serde::{de, Deserialize, Serialize};
use std::convert::TryFrom;
use std::fmt;

use crate::env::is_valid_account_id;
use crate::AccountId;

/// Helper class to validate account ID during serialization and deserializiation
/// Helper class to validate account ID during serialization and deserializiation.
/// This type wraps an [`AccountId`].
///
/// # Example
/// ```
/// use near_sdk::AccountId;
/// use near_sdk::json_types::ValidAccountId;
///
/// let id: AccountId = "bob.near".to_string();
/// let validated: ValidAccountId = id.parse().unwrap();
/// ```
Comment on lines +12 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ , examples are great, we need more of those!

#[derive(
Debug, Clone, PartialEq, PartialOrd, Ord, Eq, BorshDeserialize, BorshSerialize, Serialize,
)]
pub struct ValidAccountId(AccountId);

impl ValidAccountId {
fn is_valid(&self) -> bool {
is_valid_account_id(&self.0.as_bytes())
}
pub fn to_string(&self) -> String {
Copy link
Contributor Author

@austinabell austinabell Apr 30, 2021

Choose a reason for hiding this comment

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

Also, this function does nothing, since to_string() comes with ToString which is auto impl from fmt::Display. I kept it in though because technically it's a breaking change although I don't know if you can actually trigger it because ValidAccountId::to_string(&id) still works.

Edit: or is this to avoid some unnecessary logic around fmt::Display?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, we should remove this once we are ready to to a semver-breaking change.

self.0.clone()
}
Expand All @@ -33,35 +40,47 @@ impl AsRef<AccountId> for ValidAccountId {
}
}

impl<'de> serde::Deserialize<'de> for ValidAccountId {
fn deserialize<D>(deserializer: D) -> Result<Self, <D as serde::Deserializer<'de>>::Error>
impl<'de> Deserialize<'de> for ValidAccountId {
fn deserialize<D>(deserializer: D) -> Result<Self, <D as de::Deserializer<'de>>::Error>
where
D: serde::Deserializer<'de>,
D: de::Deserializer<'de>,
{
let s = <String as serde::Deserialize>::deserialize(deserializer)?;
s.try_into()
.map_err(|err: Box<dyn std::error::Error>| serde::de::Error::custom(err.to_string()))
let s: String = Deserialize::deserialize(deserializer)?;
Self::try_from(s).map_err(de::Error::custom)
austinabell marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl TryFrom<&str> for ValidAccountId {
type Error = Box<dyn std::error::Error>;

fn try_from(value: &str) -> Result<Self, Self::Error> {
Self::try_from(value.to_string())
value.parse()
}
}

fn validate_account_id(id: &str) -> Result<(), ParseAccountIdError> {
if is_valid_account_id(id.as_bytes()) {
Ok(())
} else {
Err(ParseAccountIdError::InvalidAccountId)
}
}

impl TryFrom<String> for ValidAccountId {
type Error = Box<dyn std::error::Error>;

fn try_from(value: String) -> Result<Self, Self::Error> {
let res = Self(value);
if res.is_valid() {
Ok(res)
} else {
Err("The account ID is invalid".into())
}
validate_account_id(value.as_str())?;
Ok(Self(value))
}
}

impl std::str::FromStr for ValidAccountId {
type Err = Box<dyn std::error::Error>;

fn from_str(value: &str) -> Result<Self, Self::Err> {
validate_account_id(value)?;
Ok(Self(value.to_string()))
}
}

Expand All @@ -71,10 +90,25 @@ impl From<ValidAccountId> for AccountId {
}
}

#[non_exhaustive]
#[derive(Debug)]
pub enum ParseAccountIdError {
InvalidAccountId,
}

impl std::fmt::Display for ParseAccountIdError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::InvalidAccountId => write!(f, "The account ID is invalid"),
}
}
}

impl std::error::Error for ParseAccountIdError {}

#[cfg(test)]
mod tests {
use super::*;
use std::convert::TryInto;

#[test]
fn test_deser() {
Expand All @@ -87,7 +121,7 @@ mod tests {

#[test]
fn test_ser() {
let key: ValidAccountId = "alice.near".try_into().unwrap();
let key: ValidAccountId = "alice.near".parse().unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

Cool, this is new to me. 👍🏼

let actual: String = serde_json::to_string(&key).unwrap();
assert_eq!(actual, "\"alice.near\"");
}
Expand Down