Filtering an Option Result using a boolean Result in Rust

Question

I'm trying to rewrite one of my pet projects in Rust, and when I was faced with the problem of filtering a Result<Option<T>, E> via a Result<bool, E>, I couldn't figure out what the best approach was.

Since I'll be doing the same thing many times throughout the project I need something that's generic while also being readable, so the solution I came up with was to implement my own traits.

My first case is to write a function that can be used to authenticate a user; by first querying the users based on the e-mail address, and then checking if the stored, encrypted password matches the plain text one retrieved from the request.

For that purpose I have the following functions:

trait AccountRepository {
  async fn find_by_email(&self, email: &String) -> Result<Option<AccountModel>, CoreApiError>;  
}

trait PasswordEncoder {
  fn verify(&self, plain: &String, encoded: &String) -> Result<bool, CoreApiError>;
}

And by combining both, I'd like to return a Result<AccountModel, CoreApiError>. When the inner Option<> is empty or the passwords don't match I'd like to return a CoreApiError that says the account was not found, and when the inner Option<> is non-empty and the passwords match, I'd like to return the AccoutModel instance that was found. If either function returns a CoreApiError I can just return it directly because I'm not really interested in it.

To that end I implemented the following traits:

pub trait FilterInner<T, F: FnOnce(&T) -> Result<bool, CoreApiError>> {
  fn filter_inner(self, f: F) -> Result<Option<T>, CoreApiError>;
}

impl<T, F: FnOnce(&T) -> Result<bool, CoreApiError>> FilterInner<T, F> for Result<Option<T>, CoreApiError> {
  fn filter_inner(self, f: F) -> Result<Option<T>, CoreApiError> {
    let result = self?;
    let filtered = match result {
      Some(data) => {
        let is_ok = f(&data)?;
        if is_ok {
          Some(data)
        } else {
          None
        }
      }

      None => None,
    };
    Ok(filtered)
  }
}

pub trait EnsurePresent<T, E>
where
  E: Into<CoreApiError>,
{
  fn ensure_present(self, message_key: &str) -> Result<T, CoreApiError>;
}

impl<T, E> EnsurePresent<T, E> for Result<Option<T>, E>
where
  E: Into<CoreApiError>,
{
  fn ensure_present(self, message_key: &str) -> Result<T, CoreApiError> {
    let maybe_data = self.map_err(|e| e.into())?;
    match maybe_data {
      Some(data) => Ok(data),

      _ => Err(CoreApiError::not_found(message_key)),
    }
  }
}

...and used them in the following fashion:

pub async fn initiate_login(&self, command: &InitiateLogin) -> Result<AccountModel, CoreApiError> {
  self
    .account_repository
    .find_by_email(&command.email)
    .await
    .filter_inner(|account| self.password_encoder_port.verify(&command.password, &account.password))
    .ensure_present("account.not.found")
}

At this point I'm pretty happy with the result, because overall I like sweeping complex stuff under the rug as long as I can provide readable and easy to use interfaces. Plus, I'd rather prefer chaining function calls than nesting match statements, so this is perfect for my needs.

That said I'm not sure if this is the best way of doing it. All throughout the process I couldn't help but think that there should be something I can use to do this easily (probably involving flat_map), but since I lack the particular Rust experience I couldn't tell what that was.

As such I'm open to any opinions, critiques, and suggestions.

Answer 1

pub async fn initiate_login(&self, command: &InitiateLogin) -> Result<AccountModel, CoreApiError> {
  self
    .account_repository
    .find_by_email(&command.email)
    .await
    .filter_inner(|account| self.password_encoder_port.verify(&command.password, &account.password))
    .ensure_present("account.not.found")
}

The challenge with this code is determining what filter_inner and ensure_present do. They aren't standard rust methods nor are they named according to a standard convention. But what's to be done about that?

One possibility is to use the ? operator to early return any errors:

pub async fn initiate_login(&self, command: &InitiateLogin) -> Result<AccountModel, CoreApiError> {
  self
    .account_repository
    .find_by_email(&command.email)
    .await?
    .try_filter(|account| self.password_encoder_port.verify(&command.password, &account.password))?
    .ok_or_else(|| CoreApiError::not_found("account.not.found"))
}

This takes care of the Results simplifying the need for special functions on Result<Object<T>>. I do invoke an imaginary try_filter method, but I think that method is simpler fits into standard rust naming conventions, it very well could be a method on Option.

Another possiblty to consider is to just use Rust's ability to do logic on patterns and don't try to make everything use higher order functions.

pub async fn initiate_login(&self, command: &InitiateLogin) -> Result<AccountModel, CoreApiError> {
  let account = self
    .account_repository
    .find_by_email(&command.email)
    .await?;

   if let Some(account) = account {
      if self.password_encoder_port.verify(&command.password, &account.password)) {
         Ok(account)
       } else {
         Err(CoreApiError::not_found("account_not_found"))
       }
   } else {
       Err(CoreApiError::not_found("account_not_found"))
   }
}

But, it seems you don't really want to use the ? operator or pattern matching.

I think I should be able to navigate within the same higher-kinded type without explicitly unwrapping it (e.g. using ? to extract the value part of a Result).

My own assessment is that the ? operator/pattern matching are handy tools, and you shouldn't be trying to avoid them. But to each their own. You could use methods on Result.

pub async fn initiate_login(&self, command: &InitiateLogin) -> Result<AccountModel, CoreApiError> {
  self
    .account_repository
    .find_by_email(&command.email)
    .await
    .and_then(|account| account.try_filter(|account| self.password_encoder_port.verify(&command.password, &account.password)))
    .and_then(|account| account.ok_or_else(|| CoreApiError::not_found("account.not.found")))
}

This avoids the ? operator and mostly sticks to the standard functions. But it is a bit messier.

Another possibility is something like NullableResult from nullable-result crate. It defines a type which combines the functionality of Option and Result.

pub async fn initiate_login(&self, command: &InitiateLogin) -> Result<AccountModel, CoreApiError> {
  self
    .account_repository
    .find_by_email(&command.email)
    .await
    .try_filter(|account| self.password_encoder_port.verify(&command.password, &account.password)))
    .result_with(|| CoreApiError::not_found("account.not.found")))
}

The methods defined on NullableResult can perform the natural actions for results of options. This means that you can just use those methods. But its a little inconvenient to have to NullableResult instead of Result<Option<>>

What you've done is defined operations on Result<Option<T>>. The tricky part to that is picking good names for the methods. It would be nice to have a sensible naming convention that made it clear. For example, iterators have try_*** methods and itertools provides ***_ok. These methods operate on iterators over Results. It'd be nice if there was good word to indicate that these methods operate over Result<Option<...>>

If you go down this route, I'd suggest having only one trait, something like ResultOption which is defined for any Result<Option<T>> and provides all of the utility methods you might want for options in results. This would replace your two separate traits for the two distinct methods.

Part of the logic of your filtering function can use the methods on result:

  fn filter_inner(self, f: F) -> Result<Option<T>, CoreApiError> {
    self.and_then(|result| match result {
             Some(data) => f(&data).map(|is_ok| if is_ok { Some(data) } else { None }),
            None => Ok(None),
    })
  }

Alternately, you could go all in on the match statements:

  fn filter_inner(self, f: F) -> Result<Option<T>, CoreApiError> {
    match self {
      Ok(Some(data)) => {
        match f(&data) {
          Err(error) => Err(error),
          Ok(true) => Ok(Some(data)),
          Ok(false) => Ok(None)
        },
      Ok(None) => Ok(None),
      Err(error) => Err(error)
    }
  }

This method seems very niche:

fn ensure_present(self, message_key: &str) -> Result<T, CoreApiError>;

Would it perhaps make more sense to have a generic method for turning Nones into errors which took a closure to define the error?