From 3c933d7940437f73a7578af7395d80b7dcbec895 Mon Sep 17 00:00:00 2001 From: 37ng Date: Wed, 10 Apr 2024 10:14:28 -0700 Subject: [PATCH] Remove PartialEq from enum errors (#641) Secondary errors will be required to implement `PartialEq` but it's not possible for external errors. If we want to compare errors, we can use different approach as showed. --- xmtp_id/src/associations/association_log.rs | 20 +++---- xmtp_id/src/associations/builder.rs | 6 +- xmtp_id/src/associations/mod.rs | 63 +++++++++------------ xmtp_id/src/associations/signature.rs | 2 +- 4 files changed, 42 insertions(+), 49 deletions(-) diff --git a/xmtp_id/src/associations/association_log.rs b/xmtp_id/src/associations/association_log.rs index d0c8dde72..809e97606 100644 --- a/xmtp_id/src/associations/association_log.rs +++ b/xmtp_id/src/associations/association_log.rs @@ -5,7 +5,7 @@ use super::state::AssociationState; use thiserror::Error; -#[derive(Debug, Error, PartialEq)] +#[derive(Debug, Error)] pub enum AssociationError { #[error("Error creating association {0}")] Generic(String), @@ -16,7 +16,7 @@ pub enum AssociationError { #[error("Signature validation failed {0}")] Signature(#[from] SignatureError), #[error("Member of kind {0} not allowed to add {1}")] - MemberNotAllowed(String, String), + MemberNotAllowed(MemberKind, MemberKind), #[error("Missing existing member")] MissingExistingMember, #[error("Legacy key is only allowed to be associated using a legacy signature with nonce 0")] @@ -165,8 +165,8 @@ impl IdentityAction for AddAssociation { // Ensure that the new member signature is correct for the new member type allowed_association( - &existing_member_identifier.kind(), - &self.new_member_identifier.kind(), + existing_member_identifier.kind(), + self.new_member_identifier.kind(), )?; let new_member = Member::new(new_member_address, Some(existing_entity_id)); @@ -347,16 +347,16 @@ fn is_legacy_signature(signature: &Box) -> bool { } fn allowed_association( - existing_member_kind: &MemberKind, - new_member_kind: &MemberKind, + existing_member_kind: MemberKind, + new_member_kind: MemberKind, ) -> Result<(), AssociationError> { // The only disallowed association is an installation adding an installation - if existing_member_kind.eq(&MemberKind::Installation) - && new_member_kind.eq(&MemberKind::Installation) + if existing_member_kind == MemberKind::Installation + && new_member_kind == MemberKind::Installation { return Err(AssociationError::MemberNotAllowed( - existing_member_kind.to_string(), - new_member_kind.to_string(), + existing_member_kind, + new_member_kind, )); } diff --git a/xmtp_id/src/associations/builder.rs b/xmtp_id/src/associations/builder.rs index 90d63a9b3..21da848c6 100644 --- a/xmtp_id/src/associations/builder.rs +++ b/xmtp_id/src/associations/builder.rs @@ -141,7 +141,7 @@ impl IdentityUpdateBuilder { } } -#[derive(Debug, Error, PartialEq)] +#[derive(Debug, Error)] pub enum SignatureRequestError { #[error("Unknown signer")] UnknownSigner, @@ -434,9 +434,9 @@ mod tests { MockSignature::new_boxed(true, rand_string().into(), SignatureKind::Erc191, None), ); - assert_eq!( + assert!(matches!( attempt_to_add_random_member, Err(SignatureRequestError::UnknownSigner) - ); + )); } } diff --git a/xmtp_id/src/associations/mod.rs b/xmtp_id/src/associations/mod.rs index 2f2680761..b6e4930f6 100644 --- a/xmtp_id/src/associations/mod.rs +++ b/xmtp_id/src/associations/mod.rs @@ -233,8 +233,7 @@ mod tests { ..Default::default() }); let update_result = apply_update(state, IdentityUpdate::new_test(vec![update])); - assert!(update_result.is_err()); - assert_eq!(update_result.err().unwrap(), AssociationError::Replay); + assert!(matches!(update_result, Err(AssociationError::Replay))); } #[test] @@ -285,11 +284,10 @@ mod tests { let state_result = get_state(vec![IdentityUpdate::new_test(vec![Action::CreateInbox( action, )])]); - assert!(state_result.is_err()); - assert_eq!( - state_result.err().unwrap(), - AssociationError::Signature(SignatureError::Invalid) - ); + assert!(matches!( + state_result, + Err(AssociationError::Signature(SignatureError::Invalid)) + )); } #[test] @@ -307,11 +305,10 @@ mod tests { initial_state.clone(), IdentityUpdate::new_test(vec![update_with_bad_existing_member]), ); - assert!(update_result.is_err()); - assert_eq!( - update_result.err().unwrap(), - AssociationError::Signature(SignatureError::Invalid) - ); + assert!(matches!( + update_result, + Err(AssociationError::Signature(SignatureError::Invalid)) + )); let update_with_bad_new_member = Action::AddAssociation(AddAssociation { new_member_signature: bad_signature.clone(), @@ -328,11 +325,10 @@ mod tests { initial_state, IdentityUpdate::new_test(vec![update_with_bad_new_member]), ); - assert!(update_result_2.is_err()); - assert_eq!( - update_result_2.err().unwrap(), - AssociationError::Signature(SignatureError::Invalid) - ); + assert!(matches!( + update_result_2, + Err(AssociationError::Signature(SignatureError::Invalid)) + )); } #[test] @@ -351,11 +347,10 @@ mod tests { }); let state_result = get_state(vec![IdentityUpdate::new_test(vec![create_request, update])]); - assert!(state_result.is_err()); - assert_eq!( - state_result.err().unwrap(), - AssociationError::MissingExistingMember - ); + assert!(matches!( + state_result, + Err(AssociationError::MissingExistingMember) + )); } #[test] @@ -383,14 +378,13 @@ mod tests { }); let update_result = apply_update(existing_state, IdentityUpdate::new_test(vec![update])); - assert!(update_result.is_err()); - assert_eq!( - update_result.err().unwrap(), - AssociationError::MemberNotAllowed( - MemberKind::Installation.to_string(), - MemberKind::Installation.to_string() - ) - ); + assert!(matches!( + update_result, + Err(AssociationError::MemberNotAllowed( + MemberKind::Installation, + MemberKind::Installation + )) + )); } #[test] @@ -569,10 +563,9 @@ mod tests { let revoke_result = apply_update(new_state, IdentityUpdate::new_test(vec![attempted_revoke])); - assert!(revoke_result.is_err()); - assert_eq!( - revoke_result.err().unwrap(), - AssociationError::MissingExistingMember - ); + assert!(matches!( + revoke_result, + Err(AssociationError::MissingExistingMember) + )); } } diff --git a/xmtp_id/src/associations/signature.rs b/xmtp_id/src/associations/signature.rs index d71bec608..ae71b83f5 100644 --- a/xmtp_id/src/associations/signature.rs +++ b/xmtp_id/src/associations/signature.rs @@ -2,7 +2,7 @@ use thiserror::Error; use super::MemberIdentifier; -#[derive(Debug, Error, PartialEq)] +#[derive(Debug, Error)] pub enum SignatureError { #[error("Signature validation failed")] Invalid,