3
\$\begingroup\$

I am trying to puzzle out api based user registration for a service I am building, but I have a strong feeling that my code is not optimal.

It feels like passing "state" in the way I did prevents the server from being multithreaded, as I have no clue how actix could share data that isn't Send or Sync across multiple threads.

I also interact with DynamoDb by using a .sync() call on the request, which means that i block on waiting for Dynamo to respond to me. This seems like it would be bad for performance of a web server but I don't know how to avoid it without something like async/await.

Another issue is that I cannot guarantee that a user has a unique email without making a second table in Dynamo. I don't want to expose a user's email to the outside world, so for everything else I would need to use some sort of other unique identifier (here I went with a uuid), but for registration I would still need to check that requirement...somehow. But if I make a second table I introduce the possibility of an inconsistent state since to my knowledge I can't make dynamo affect two tables atomically.

I also just returned out the created user id to the client, but is there something more useful I could have done? Should I return a token in this response? More info about the created user?

Any and all feedback would be greatly appreciated.

use actix_web::error::InternalError;
use actix_web::http::StatusCode;
use actix_web::{http, server, App, Json, Responder, State};
use bcrypt;
use core::borrow::Borrow;
use core::fmt;
use failure::Error;
use hocon::HoconLoader;
use log::{info, warn, Level};
use maplit::hashmap;
use rusoto_core::{Region, RusotoError};
use rusoto_dynamodb::{
    AttributeDefinition, AttributeValue, BatchWriteItemInput, CreateTableInput, DynamoDb,
    DynamoDbClient, GetItemInput, GetItemOutput, KeySchemaElement, ProvisionedThroughput,
    PutItemInput, PutRequest, WriteRequest,
};
use serde::{Deserialize, Serialize};
use serde_dynamodb;
use simple_logger;
use std::collections::HashMap;
use std::fmt::{Debug, Formatter};
use std::str::FromStr;
use uuid::Uuid;

type DynamoRecord = HashMap<String, AttributeValue>;

/// The info we need from a person in order to create a user for them.
#[derive(Serialize, Deserialize)]
pub struct UserRegistrationInfo {
    email: String,
    password: String,
}

impl Debug for UserRegistrationInfo {
    fn fmt(&self, f: &mut Formatter) -> fmt::Result {
        write!(
            f,
            "UserRegistrationInfo {{ email = {:?}, password = <SENSITIVE> }}",
            self.email
        )
    }
}

/// The canonical user record that is actually stored in the database.
#[derive(Serialize, Deserialize, Debug)]
pub struct User {
    id: String,
    email: String,
    hashed_password: String,
}

impl User {
    /// Creates a User from the bare minimum info we received from the user. Note that
    /// this function is not pure as it involves at the very least generation of a unique
    /// id for the user.
    pub fn from_registration_info(registration_info: UserRegistrationInfo) -> User {
        User {
            id: Uuid::new_v4().to_string(),
            email: registration_info.email,
            hashed_password: bcrypt::hash(registration_info.password, bcrypt::DEFAULT_COST)
                .unwrap(),
        }
    }

    fn to_dynamo(&self) -> serde_dynamodb::error::Result<DynamoRecord> {
        Ok(serde_dynamodb::to_hashmap(self)?)
    }

    fn from_dynamo(m: DynamoRecord) -> serde_dynamodb::error::Result<User> {
        serde_dynamodb::from_hashmap(m)
    }
}

pub struct System {
    pub config: Config,
    pub dynamo_db: DynamoDbClient,
}

impl System {
    pub fn from_config(config: Config) -> System {
        let dynamo_db = DynamoDbClient::new(config.aws_region());
        System { config, dynamo_db }
    }
}

/// The data returned to the user in the event that they successfully register a user
#[derive(Serialize)]
struct SuccessfulRegistration {
    user_id: String,
}

fn register(
    registration_info: Json<UserRegistrationInfo>,
    state: State<System>,
) -> Result<impl Responder, failure::Error> {
    info!("Creating a new user");
    let new_user: User = User::from_registration_info(registration_info.into_inner());
    let put_result = state
        .dynamo_db
        .put_item(PutItemInput {
            table_name: String::from("chat_users"),
            item: new_user.to_dynamo().unwrap(),
            ..PutItemInput::default()
        })
        .sync()?;

    state
        .dynamo_db
        .batch_write_item(BatchWriteItemInput {
            request_items: hashmap! {
                state.config.users_table() => vec![
                    WriteRequest {
                        put_request: Some(PutRequest {
                            item: new_user.to_dynamo().unwrap()
                        }),
                        ..WriteRequest::default()
                    }
                ],
                state.config.login_table() => vec![
                    WriteRequest {
                        put_request: Some(PutRequest {
                            item: new_user.to_dynamo().unwrap()
                        }),
                        ..WriteRequest::default()
                    }
                ]
            },
            ..BatchWriteItemInput::default()
        })
        .sync()?;

    info!("Successfully created a new user");
    Ok(actix_web::HttpResponse::Ok().json(SuccessfulRegistration {
        user_id: new_user.id,
    }))
}

/// Creates the user table. Note: Dynamo doesn't have a "create if not exists" so this needs
/// to be externalized to a good ol' cloudformation script or similar.
fn create_tables(
    client: &DynamoDbClient,
) -> rusoto_core::RusotoResult<(), rusoto_dynamodb::CreateTableError> {
    client
        .create_table(CreateTableInput {
            attribute_definitions: vec![AttributeDefinition {
                attribute_name: "id".to_string(),
                attribute_type: "S".to_string(),
            }],
            provisioned_throughput: Some(ProvisionedThroughput {
                read_capacity_units: 5,
                write_capacity_units: 5,
            }),
            key_schema: vec![KeySchemaElement {
                attribute_name: "id".to_string(),
                key_type: "HASH".to_string(),
            }],
            table_name: "chat_users".to_string(),
            ..CreateTableInput::default()
        })
        .sync()?;
    Ok(())
}

#[derive(Debug, Clone)]
pub struct Config {
    aws_region: Region,
    users_table: String,
    login_table: String,
}

impl Config {
    pub fn aws_region(&self) -> Region {
        self.aws_region.clone()
    }

    pub fn users_table(&self) -> String {
        self.users_table.clone()
    }

    pub fn login_table(&self) -> String {
        self.login_table.clone()
    }

    fn load() -> Result<Config, failure::Error> {
        let hocon = HoconLoader::new()
            .load_file("./application.conf")?
            .hocon()?;

        let aws_region = match hocon["aws"]["region"]
            .as_string()
            .as_ref()
            .map(String::as_str)
        {
            Some("local") => {
                info!("Using \"local\" as our AWS region");
                Region::Custom {
                    name: "local".to_string(),
                    endpoint: "http://localhost:8000".to_string(),
                }
            }
            Some(region) => Region::from_str(region).unwrap_or_else(|_| {
                warn!("Unknown region: {:?}, defaulting to us-west-2", region);
                Region::UsWest2
            }),

            None => {
                warn!("No region provided, defaulting to us-west-2");
                Region::UsWest2
            }
        };

        let users_table = hocon["dynamo"]["users_table"].as_string().unwrap();
        let login_table = hocon["dynamo"]["login_table"].as_string().unwrap();

        Ok(Config {
            aws_region,
            users_table,
            login_table,
        })
    }
}

fn main() -> Result<(), Box<std::error::Error>> {
    simple_logger::init_with_level(Level::Info).unwrap();

    let config = Config::load()?;
    server::new(move || {
        App::with_state(System::from_config(config.clone())).route(
            "/register",
            http::Method::POST,
            register,
        )
    })
    .bind("127.0.0.1:8080")
    .unwrap()
    .run();

    Ok(())
}
```
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Browse other questions tagged or ask your own question.