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

Added support for serde. #6

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

Wopple
Copy link

@Wopple Wopple commented Oct 8, 2021

We would like to be able to send validated email addresses between servers. This pull request adds serde support to EmailAddress so it can be communicated across the wire in a modeled way. The (de)serialization assumes strict mode. The "serde" feature is optional and off by default.

Versions:

serde = "1.0.115"
serde_json = "1.0.57"

were used because they were preexisting in the lock file and the latest patch versions caused compilation errors.

I restricted the existing editor config to non-rust files because intellij was using 2-space tabs in rust files when 4 is desired.

@Sayan751
Copy link
Owner

Sayan751 commented Oct 9, 2021

Thank you for the PR. However, I am not sure how to feel about the changes. As far as I understand, the de-/serialization can also be derived for remote crate using serde: https://serde.rs/remote-derive.html.

@Wopple
Copy link
Author

Wopple commented Oct 11, 2021

I believe the behavior can be overridden by using the same remote derive functionality if they don't want this behavior. It's also behind an off-by-default feature flag so they don't have to bring it in to their code.

The questions I see are:

  • does the default implementation want to run in strict or lenient mode?
  • does the default implementation want to represent the email address as a single string or two parts?

This pull request answers those with strict, and two parts. Considering #4 I think strict is a reasonable assumption here. I chose two parts because I wanted to mirror the struct type.

@Sayan751
Copy link
Owner

@Wopple The default parsing mode is always strict. That is while instantiating, if you pass None as the options, then it is parsed strictly. If you want to parse in lax mode, you need to set the lax flag explicitly to true.

The reason I am not keen on deriving the de-/serialization behavior is that I feel that the de-/serialization behavior (serialized format) should be client specific, and it does not sounds like a nice idea to end up eventually implementing every possible format out there. Moreover, I don't want to tie this lib with any specific serialization library.

@Wopple
Copy link
Author

Wopple commented Oct 11, 2021

I think most people using a library want it to "just work" without having to think about the details. That's the situation I'm in, I want to serialize with modeled emails, the specific representation doesn't matter to me. The specific implementation only matters in marginal cases where every byte matters. In those marginal cases, they can still do that since this is an optional feature flag and that remote derivation exists.

As for tying to the Serde library, I think it's reasonable. Serde has made it onto this checklist of writing libraries:

https://rust-lang.github.io/api-guidelines/checklist.html

So I think it's achieved "standard" status as a library. Additionally, if a new de-serialization library takes over, it's still just a feature flag. That new library could be implemented with a different feature flag.

@Wopple
Copy link
Author

Wopple commented Oct 12, 2021

I don't want you to feel like I'm pressuring you, I can move forward without this being merged using branch dependencies. So if you think this needs more thought and care that's fine.

@azizghuloum
Copy link

azizghuloum commented Mar 9, 2023

If somebody else comes here for serde support, this is what I've done:

Copy this code into email_address_serde.rs:

use std::{fmt, marker::PhantomData};

use email_address_parser::EmailAddress;
use serde::{de, Deserializer, Serialize, Serializer};

pub fn serialize<S: Serializer>(email: &EmailAddress, serializer: S) -> Result<S::Ok, S::Error> {
  email.to_string().serialize(serializer)
}

struct Visitor<T: ?Sized>(PhantomData<T>);

impl<'a> de::Visitor<'a> for Visitor<EmailAddress> {
  type Value = EmailAddress;

  fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
    formatter.write_str("an email address as `string`")
  }

  fn visit_str<E: de::Error>(self, value: &str) -> Result<EmailAddress, E> {
    EmailAddress::parse(value, None).ok_or(E::custom("invalid email format"))
  }
}

pub fn deserialize<'a, D: Deserializer<'a>>(deserializer: D) -> Result<EmailAddress, D::Error> {
  deserializer.deserialize_str(Visitor::<EmailAddress>(PhantomData))
}

Then in your main, add:

mod email_address_serde;

and use it like so:

#[derive(Debug, Serialize, Deserialize)]
struct Login {
  #[serde(with = "email_address_serde")]
  email: EmailAddress,
  password: String,
}

@kaduvert
Copy link

kaduvert commented Jul 5, 2023

if the author had merged this, i wouldn't have spent half an hour on email serialization.

thanks @azizghuloum for the workaround

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants