Stricter types

.config/extra.dic

@@ -23,3 +23,4 @@ d2
 incrementing
 findable
 deserializer
+deserialization

src/id.rs

@@ -18,23 +18,45 @@ use serde::{de::Visitor, Deserialize, Deserializer, Serialize, Serializer};
 macro_rules! add_id {
 	(
 		$(#[$meta:meta])*
-		$name:ident
+		$name:ident,
+		$id_matches:expr
 	) => {
 		#[doc = concat!("An ID of a VRC ", stringify!($name))]
 		///
 		/// # Example usage
 		///
+		/// Note that parse checks that the ID follows the correct format:
+		///
+		/// ```
+		#[doc = concat!("use vrc::id::", stringify!($name), ";")]
+		#[doc = concat!("let parse_res = \"totally-legit-id\".parse::<", stringify!($name), ">();")]
+		/// assert!(parse_res.is_err());
+		/// ```
+		///
+		/// But parsing checks can also be ignored by using infallible `From<String>` implementations:
+		///
 		/// ```
 		#[doc = concat!("use vrc::id::", stringify!($name), ";")]
-		#[doc = concat!("let id1 = \"totally-legit-id\".parse::<", stringify!($name), ">().unwrap();")]
-		#[doc = concat!("let id2 = \"other-totally-legit-id\".parse::<", stringify!($name), ">().unwrap();")]
+		/// // Note that parsing will fail with the wrong format
+		#[doc = concat!("let id1 = ", stringify!($name), "::from(\"totally-legit-id\");")]
+		#[doc = concat!("let id2 = ", stringify!($name), "::from(\"other-totally-legit-id\");")]
 		/// assert!(id1 != id2);
 		/// ```
-		#[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
+		///
+		/// The deserialization also checks the that the IDs format is valid, whilst serialization does not check the validity.
+		#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)]
 		#[repr(transparent)]
 		$(#[$meta])*
 		pub struct $name(String);
 
+		impl $name {
+			/// Checks if the ID matches the expected format
+			#[must_use]
+			pub fn is_valid(&self) -> bool {
+				$id_matches(&self.0)
+			}
+		}
+
 		impl AsRef<str> for $name {
 			/// Extracts a string slice containing the entire inner String.
 			#[must_use]
@@ -54,6 +76,9 @@ macro_rules! add_id {
 			type Err = &'static str;
 
 			fn from_str(id: &str) -> Result<Self, Self::Err> {
+				if !$id_matches(&id) {
+					return Err(concat!("ID doesn't match expected format"))
+				}
 				Ok(Self(id.to_owned()))
 			}
 		}
@@ -83,15 +108,57 @@ macro_rules! add_id {
 				Any::$name(id)
 			}
 		}
+
+		/// The deserializer will give an error if the inner String doesn't start with
+		/// the proper prefix.
+		impl<'de> serde::de::Deserialize<'de> for $name {
+			fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+			where
+				D: Deserializer<'de>,
+			{
+				struct IdVisitor;
+
+				impl<'de> Visitor<'de> for IdVisitor {
+					type Value = $name;
+
+					fn expecting(
+						&self, formatter: &mut std::fmt::Formatter,
+					) -> std::fmt::Result {
+						formatter
+							.write_str(concat!("a string UD if", stringify!($name), ", that is of the correct format"))
+					}
+
+					fn visit_str<E>(self, v: &str) -> Result<Self::Value, E>
+					where
+						E: serde::de::Error,
+					{
+						if !$id_matches(&v) {
+							return Err(serde::de::Error::invalid_value(
+								serde::de::Unexpected::Str(v),
+								&"not matching the ID format",
+							));
+						}
+
+						Ok($name(v.to_string()))
+					}
+				}
+
+				deserializer.deserialize_str(IdVisitor)
+			}
+		}
 	};
 }
 
-add_id!(Avatar);
-add_id!(Group);
-add_id!(Instance);
-add_id!(UnityPackage);
-add_id!(User);
-add_id!(World);
+// VRC still has some legacy IDs, thus allowing 10 char strings without prefix
+add_id!(Avatar, |v: &str| v.starts_with("avtr_") || v.len() == 10);
+add_id!(Group, |v: &str| v.starts_with("grp_"));
+// TODO: Manual implementation that breaks down instance name, type, region, and
+// so on.
+add_id!(Instance, |v: &str| v.contains("~region("));
+add_id!(UnityPackage, |v: &str| v.starts_with("unp_") || v.len() == 10);
+add_id!(User, |v: &str| v.starts_with("usr_") || v.len() == 10);
+add_id!(GroupMember, |v: &str| v.starts_with("gmem_"));
+add_id!(World, |v: &str| v.starts_with("wrld_") || v.len() == 10);
 
 /// Offline or the id of the world or whatever type T is
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
@@ -144,9 +211,12 @@ impl<T> OfflineOrPrivateOr<T> {
 /// # Example usage
 ///
 /// ```
-/// let id1 = "totally-legit-id".parse::<vrc::id::User>().unwrap();
+/// let id1 = "usr_totally-legit-uuid".parse::<vrc::id::User>().unwrap();
 /// let id1: vrc::id::Any = id1.into();
-/// let id2 = "totally-legit-id".parse::<vrc::id::Instance>().unwrap();
+/// let id2 =
+/// 	"0000~group(grp_totally-legit-uuid)~groupAccessType(public)~region(us)"
+/// 		.parse::<vrc::id::Instance>()
+/// 		.unwrap();
 /// let id2: vrc::id::Any = id2.into();
 /// assert!(id1 != id2);
 /// ```
@@ -165,6 +235,8 @@ pub enum Any {
 	User(User),
 	/// A world ID
 	World(World),
+	/// A group member ID
+	GroupMember(GroupMember),
 }
 
 impl AsRef<str> for Any {
@@ -178,6 +250,7 @@ impl AsRef<str> for Any {
 			Self::UnityPackage(v) => v.as_ref(),
 			Self::User(v) => v.as_ref(),
 			Self::World(v) => v.as_ref(),
+			Self::GroupMember(v) => v.as_ref(),
 		}
 	}
 }
@@ -257,6 +330,25 @@ impl<'de> serde::de::Deserialize<'de> for WorldInstance {
 	}
 }
 
+#[cfg(test)]
+#[test]
+fn user_id_parsing() {
+	// Tupper
+	let id = "\"usr_c1644b5b-3ca4-45b4-97c6-a2a0de70d469\"";
+	assert!(serde_json::from_str::<crate::id::User>(id).is_ok());
+
+	let id = "\"grp_c1644b5b-3ca4-45b4-97c6-a2a0de70d469\"";
+	assert!(serde_json::from_str::<crate::id::User>(id).is_err());
+
+	// Valid length old user ID
+	let id = "\"qYZJsbJRqA\"";
+	assert!(serde_json::from_str::<crate::id::User>(id).is_ok());
+
+	// Invalid length
+	let id = "\"qYZJsbJRqA1\"";
+	assert!(serde_json::from_str::<crate::id::User>(id).is_err());
+}
+
 #[cfg(test)]
 #[test]
 fn world_and_instance() {
@@ -267,3 +359,35 @@ fn world_and_instance() {
 		serde_json::to_string(&id).expect("to be able to serialize WorldInstance");
 	assert_eq!(original_id, id);
 }
+
+#[cfg(test)]
+#[test]
+fn strict_from_string() {
+	use std::str::FromStr;
+
+	let original_id = "\"grp_93451756-8327-4ecc-b978-3e60aa9f64a9\"";
+	let id: crate::id::Group =
+		serde_json::from_str(original_id).expect("to be able to deserialize Group");
+	let id: String =
+		serde_json::to_string(&id).expect("to be able to serialize a valid Group");
+	assert_eq!(original_id, id);
+
+	let original_id = "\"93451756-8327-4ecc-b978-3e60aa9f64a9\"";
+
+	// For now, deserialization is still allowed with invalid IDs
+	//assert!(serde_json::from_str::<crate::id::Group>(original_id).is_err(),
+	// "deserializing an invalid Group ID errors");
+	assert!(
+		crate::id::Group::from_str(original_id).is_err(),
+		"from_str for an invalid Group ID errors"
+	);
+	// From<String> implementations are infallible, which should always should
+	// work...
+	let id = crate::id::Group::from(original_id);
+	assert!(
+		!id.is_valid(),
+		"Force converted group ID can be detected as invalid"
+	);
+	let _id: String = serde_json::to_string(&id)
+		.expect("to be able to serialize an invalid Group");
+}

src/model/groups.rs

@@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize};
 use serde_json::Value;
 use time::{serde::rfc3339, OffsetDateTime};
 
-use crate::id::User;
+use crate::id;
 
 #[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
@@ -79,11 +79,11 @@ pub struct GroupAuditLog {
 	/// The unique identifier of the group associated with the audit log.
 	pub group_id: String,
 	/// The unique identifier of the actor who performed the action.
-	pub actor_id: User,
+	pub actor_id: id::User,
 	/// The display name of the actor.
 	pub actor_displayname: Option<String>,
 	/// The unique identifier of the target of the action.
-	pub target_id: Option<User>,
+	pub target_id: Option<id::User>,
 	/// The type of event captured in the audit log.
 	pub event_type: String,
 	/// The description of the event captured in the audit log.
@@ -117,77 +117,99 @@ pub struct GroupAuditLogDataChange<T> {
 	pub new: T,
 }
 
+// TODO: Merge `GroupBan` amd `GroupMember` common fields
+
 #[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
 /// Details about a group ban/un-ban.
 pub struct GroupBan {
-	/// Unique identifier of the ban.
-	pub id: String,
+	/// Identifier of the group member
+	pub id: id::GroupMember,
 	/// Identifier of the group.
-	pub group_id: crate::id::Group,
+	pub group_id: id::Group,
 	/// Identifier of the user who was banned.
-	pub user_id: crate::id::User,
+	pub user_id: id::User,
 	/// Flag indicating if the user was representing the group at the time of
 	/// ban.
 	pub is_representing: bool,
 	/// List of role identifiers the user had in the group.
 	pub role_ids: Vec<Value>,
+	// TODO: Rename
 	/// List of managed role identifiers the user had in the group.
 	pub m_role_ids: Vec<Value>,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// Time of when the user joined.
-	pub joined_at: Option<String>,
+	pub joined_at: Option<OffsetDateTime>,
 	/// Status of the user's membership in the group at the time of ban.
 	pub membership_status: String,
 	/// Visibility status of the user in the group.
 	pub visibility: String,
 	/// Flag indicating if the user was subscribed to group announcements.
 	pub is_subscribed_to_announcements: bool,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// Time of the last post read by the user in the group.
-	pub last_post_read_at: Option<Value>,
+	pub last_post_read_at: Option<OffsetDateTime>,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// Time when the user joined the group.
-	pub created_at: String,
+	pub created_at: Option<OffsetDateTime>,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// Time when the user was banned from the group.
-	pub banned_at: Option<String>,
+	pub banned_at: Option<OffsetDateTime>,
+	#[serde(default)]
 	/// Notes added by the group manager regarding the ban.
 	pub manager_notes: String,
 	/// Flag indicating if the user joined the group from a purchase.
 	pub has_joined_from_purchase: bool,
 }
 
+// TODO: Split limited group member away from what admin endpoint gives
 #[derive(Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)]
 #[serde(rename_all = "camelCase")]
 /// Details about a group member.
 pub struct GroupMember {
 	/// Unique identifier for the group member
-	pub id: String,
+	pub id: id::GroupMember,
 	/// Identifier for the group
-	pub group_id: String,
+	pub group_id: id::Group,
 	/// Identifier for the user
-	pub user_id: String,
+	pub user_id: id::User,
 	/// This field indicates whether the user is representing the group or not
 	pub is_representing: bool,
 	/// List of role identifiers associated with the user in the group
 	pub role_ids: Vec<String>,
 	/// List of manager role identifiers associated with the user in the group
 	pub m_role_ids: Vec<String>,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// The date and time when the user joined the group
-	pub joined_at: Option<String>,
+	pub joined_at: Option<OffsetDateTime>,
 	/// The status of the user's membership in the group
 	pub membership_status: String,
+	// TODO: Enum
 	/// The visibility status of the user in the group
 	pub visibility: String,
 	/// This field indicates whether the user is subscribed to group
 	/// announcements or not
 	pub is_subscribed_to_announcements: bool,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// The date and time of the last post read by the user
-	pub last_post_read_at: Option<String>,
+	pub last_post_read_at: Option<OffsetDateTime>,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// The date and time when the group member was created
-	pub created_at: String,
+	pub created_at: Option<OffsetDateTime>,
+	#[serde(default)]
+	#[serde(with = "rfc3339::option")]
 	/// The date and time when the user was banned from the group, if applicable
-	pub banned_at: Option<String>,
+	pub banned_at: Option<OffsetDateTime>,
 	/// Notes made by the manager about the user
-	pub manager_notes: String,
+	pub manager_notes: Option<String>,
 	/// This field indicates whether the user has joined the group from a
 	/// purchase or not
-	pub has_joined_from_purchase: bool,
+	pub has_joined_from_purchase: Option<bool>,
 }

src/query/groups.rs

@@ -112,6 +112,7 @@ impl Queryable<Authentication, crate::model::GroupBan> for GroupUnban {
 }
 
 /// Returns a Limited Group Member.
+/// Will error without proper permissions, if not a member, etc.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, Deserialize, Serialize)]
 pub struct GroupMember {
 	/// The ID of the group
@@ -120,9 +121,7 @@ pub struct GroupMember {
 	pub user_id: crate::id::User,
 }
 
-impl Queryable<Authentication, Option<crate::model::GroupMember>>
-	for GroupMember
-{
+impl Queryable<Authentication, crate::model::GroupMember> for GroupMember {
 	fn url(&self, _: &Authentication) -> String {
 		format!(
 			"{}/groups/{}/members/{}",

src/query/mod.rs

@@ -19,7 +19,7 @@ mod worlds;
 pub use worlds::*;
 
 #[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
-/// [`racal::Queryable`](racal::Queryable)'s `RequiredApiState`.
+/// [`racal::Queryable`]'s `RequiredApiState`.
 ///
 /// Even unauthenticated requests to VRC's API should take rate limits
 /// into account, thus not using `()` for the API state.
@@ -44,7 +44,7 @@ impl racal::FromApiState<Self> for Authenticating {
 }
 
 #[derive(Clone, PartialEq, Eq, Hash, Serialize, Deserialize)]
-/// [`racal::Queryable`](racal::Queryable)'s `RequiredApiState`.
+/// [`racal::Queryable`]'s `RequiredApiState`.
 ///
 /// With authentication
 pub struct Authentication {