refactor(types): prefer EmailAddress type to raw strings #196
Conversation
Ok(_) => assert!(false, "DbClient::get_bounces should have failed"), | ||
Err(error) => assert_eq!(format!("{}", error), "400 Bad Request"), | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here we can delete the entire test case because it's not possible to construct an invalid EmailAddress
instance to pass to db.get_bounces
.
return false; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, all this ugly stuff can go now because the validation is already encapsulated inside EmailAddress
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, though like other Rust PRs, someone with more rust chops should have a look too.
The EmailAddress type was only being used in some places, which meant some places that weren't using it had to call `validate::email_address` manually. This change spreads it throughout most of the project, so that almost everywhere gets to benefit from strong typing. The only place I decided to leave it alone was the provider layer, where some providers have their own `EmailAddress` struct. I could have aliased it at that layer too, but by that point addresses are just dumb strings anyway so it didn't seem worth it.
e49fce8
to
dc08356
Compare
Merging anyway, because I have a queue of PRs that are partially blocked by this one and it's not clear if anyone else is going to review it or not. If anyone does have further feedback on this PR, feel free to mention it here and I can address it separately. |
The EmailAddress type was only being used in some places, which meant some places that weren't using it had to call
validate::email_address
manually. This change spreads it throughout most of the project, so that almost everywhere gets to benefit from strong typing.The only place I decided to leave alone was the provider layer, where some providers have their own
EmailAddress
struct. I could have aliased it at that layer too, but by that point addresses are just dumb strings anyway so it didn't seem worth it.(this is an extraction from
pb/166-wip
because that branch is growing too big)@mozilla/fxa-devs r?