Skip to content

Commit

Permalink
Remove PartialEq from enum errors (#641)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
37ng authored Apr 10, 2024
1 parent 1a3105b commit 3c933d7
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 49 deletions.
20 changes: 10 additions & 10 deletions xmtp_id/src/associations/association_log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand All @@ -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")]
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -347,16 +347,16 @@ fn is_legacy_signature(signature: &Box<dyn Signature>) -> 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,
));
}

Expand Down
6 changes: 3 additions & 3 deletions xmtp_id/src/associations/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ impl IdentityUpdateBuilder {
}
}

#[derive(Debug, Error, PartialEq)]
#[derive(Debug, Error)]
pub enum SignatureRequestError {
#[error("Unknown signer")]
UnknownSigner,
Expand Down Expand Up @@ -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)
);
));
}
}
63 changes: 28 additions & 35 deletions xmtp_id/src/associations/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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(),
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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)
));
}
}
2 changes: 1 addition & 1 deletion xmtp_id/src/associations/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 3c933d7

Please sign in to comment.