0

I have a codebase that I am working on in Rust and I intend to have sync and async versions available through a feature flag, as I thought that was the best way to make sure the whole ecosystem remained consistent. That way I don't have to do weird shenanigans in order to convert from async to sync code, however doing this has been incredibly tedious and it is getting more difficult to change small things in the code. How would I reduce the amount of duplicated code here?

There are many functions like this, the only difference being that the functions that are async from the async flag are being .await -ed afterwards

#[cfg(feature = "async")]
    pub async fn new_from_env(realm_id: &str, environment: Environment) -> Result<Self, AuthError> {
        let discovery_doc = Self::get_discovery_doc(&environment).await?; // Only difference
        let client_id = ClientId::new(std::env::var("INTUIT_CLIENT_ID")?);
        let client_secret = ClientSecret::new(std::env::var("INTUIT_CLIENT_SECRET")?);
        let redirect_uri = RedirectUrl::new(std::env::var("INTUIT_REDIRECT_URI")?)?;
        log::info!("Got Discovery Doc and Intuit Credentials Successfully");
        Ok(Self {
            redirect_uri,
            realm_id: realm_id.to_string(),
            environment,
            data: Unauthorized {
                client_id,
                client_secret,
                discovery_doc,
            },
        })
    }

    #[cfg(not(feature = "async"))]
    pub async fn new_from_env(realm_id: &str, environment: Environment) -> Result<Self, AuthError> {
        let discovery_doc = Self::get_discovery_doc(&environment)?; // No await
        let client_id = ClientId::new(std::env::var("INTUIT_CLIENT_ID")?);
        let client_secret = ClientSecret::new(std::env::var("INTUIT_CLIENT_SECRET")?);
        let redirect_uri = RedirectUrl::new(std::env::var("INTUIT_REDIRECT_URI")?)?;
        log::info!("Got Discovery Doc and Intuit Credentials Successfully");
        Ok(Self {
            redirect_uri,
            realm_id: realm_id.to_string(),
            environment,
            data: Unauthorized {
                client_id,
                client_secret,
                discovery_doc,
            },
        })
    }

#[cfg(feature = "async")]
    async fn default_grab_token_session(
        client_ref: &BasicClient,
        scopes: Option<&[IntuitScope]>,
    ) -> Result<TokenSession, AuthError> {
        let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256();
        let (auth_url, csrf_state) = Self::get_auth_url(client_ref, pkce_challenge, scopes);
        let listener = TcpListener::bind("127.0.0.1:3320")
            .await // Here
            .expect("Error starting localhost callback listener! (async)");
        open::that_detached(auth_url.as_str())?;
        log::info!("Opened Auth URL: {}", auth_url);
        Self::handle_oauth_callback(client_ref, listener, csrf_state, pkce_verifier).await
    }

    #[cfg(not(feature = "async"))]
    fn default_grab_token_session(
        client_ref: &BasicClient,
        scopes: Option<&[IntuitScope]>,
    ) -> Result<TokenSession, AuthError> {
        let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256();
        let (auth_url, csrf_state) = Self::get_auth_url(client_ref, pkce_challenge, scopes);
        let listener = TcpListener::bind("127.0.0.1:3320")
            .expect("Error starting localhost callback listener!"); // Not here
        open::that_detached(auth_url.as_str())?;
        log::info!("Opened Auth URL: {}", auth_url);
        Self::handle_oauth_callback(client_ref, listener, csrf_state, pkce_verifier)
    }

In order to reduce the amount of time just doing any changes to one part of the code in two locations any time, there must be a simple and clean way to do this that I'm missing

I tried using the duplicate crates and writing a macro_rules! macro for it, but neither solution was simple to use because there's not simple way of determining whether or not a type is available to be awaited

example:

#[cfg(feature = "async")]
use tokio::fs;
#[cfg(not(feature = "async"))]
use std::fs;

macro_rules! cfg_async {
    ($func_name:ident ($($args:ident: $state_ty:ty),*) -> $output:ident { $body:expr } ) => {
      #[cfg(feature = "async")]
      async fn $func_name($($args: $state_ty),*) -> $output {
         // If object can be awaited, await it, otherwise stay the same
         $body
      }
  
      #[cfg(not(feature = "async"))]
      fn $func_name($($args: $state_ty),*) -> $output {
         // All the functions are synchronous, nothing needs to be awaited
         $body
      }
    };
}


cfg_async!(foo (path: &str) -> String {
    fs::read_to_string(path).unwrap()
    // In async should be fs::read_to_string("foo.txt").await.unwrap()
});

How else can I go about this?

  • It's probably easier to go the other way round, rather than adding `.await` where necessary you could take the asynchronous version of your function and strip it off `async` and `await` that'll be much easier since you don't have to some how detect what needs awaiting and what doesn't which I don't think is possible. – cafce25 Aug 15 '23 at 04:24
  • Note that feature flags are designed to be additive. They can guard optional code that is only needed in some cases, and thereby reduce compile times, binary sizes and dependencies. However, they don't work well for choosing between two options, since Cargo will merge the features enabled for a dependency if it occurs multiple times in the dependency graph. That means even if I use your crate with the `async` feature disabled in my Cargo.toml file, some other crate in my dependency graph might also use your crate with `async` enabled, in which case it would also be enabled for me. – Sven Marnach Aug 15 '23 at 07:25

1 Answers1

2

This is a known problem. You can read about it in this Inside Rust blog post: Announcing the Keyword Generics Initiative. That post mentions maybe-async, which you might want to try, but I haven't checked it out.

Since most of your function is sync, you can move that to its own function to reduce duplication.

#[cfg(feature = "async")]
pub async fn new_from_env(realm_id: &str, environment: Environment) -> Result<Self, AuthError> {
    let discovery_doc = Self::get_discovery_doc(&environment).await?; // Only difference
    Self::new_from_env_internal(realm_id, environment, discovery_doc)
}

#[cfg(not(feature = "async"))]
pub fn new_from_env(realm_id: &str, environment: Environment) -> Result<Self, AuthError> {
    let discovery_doc = Self::get_discovery_doc(&environment)?; // No await
    Self::new_from_env_internal(realm_id, environment, discovery_doc)
}

fn new_from_env_internal(
    realm_id: &str,
    environment: Environment,
    discovery_doc: DiscoveryDoc,
) -> Result<Self, AuthError> {
    let client_id = ClientId::new(std::env::var("INTUIT_CLIENT_ID")?);
    let client_secret = ClientSecret::new(std::env::var("INTUIT_CLIENT_SECRET")?);
    let redirect_uri = RedirectUrl::new(std::env::var("INTUIT_REDIRECT_URI")?)?;
    log::info!("Got Discovery Doc and Intuit Credentials Successfully");
    Ok(Self {
        redirect_uri,
        realm_id: realm_id.to_string(),
        environment,
        data: Unauthorized {
            client_id,
            client_secret,
            discovery_doc,
        },
    })
}

Keeping this up throughout your codebase may or may not be easy, but generics and reorganization can help.

Note that using features like this is not advisable, since Cargo intends for features to be additive. If someone else tries to depend on both the sync and async parts of your crate, they will find that the sync API is not available, and there is no clean way of getting both. Often this is solved by separating the sync and async parts into different modules, such as in reqwest, but this also creates extra wrapper functions and namespace organization. The other solution is to give the async and sync functions different names, but this is usually only used on a small scale, like recv and blocking_recv in Tokio.

drewtato
  • 6,783
  • 1
  • 12
  • 17