refactor(redis): extract redis code to a dedicated db module

API.md

@@ -24,7 +24,7 @@
 - `code: 429, errno: 106`: Bounce Complaint Error
 - `code: 429, errno: 107`: Bounce Soft Error
 - `code: 429, errno: 108`: Bounce Hard Error
-- `code: 500, errno: 109`: Database Error
+- `code: 500, errno: 109`: AuthDb Error
 - `code: 500, errno: 110`: Queue Error
 - `code: 500, errno: 111`: Invalid Notification Type
 - `code: 500, errno: 112`: Missing Notification Payload
@@ -32,8 +32,9 @@
 - `code: 500, errno: 114`: SQS Message Hash Mismatch
 - `code: 500, errno: 115`: SQS Message Parsing Error
 - `code: 500, errno: 116`: Duration Error
-- `code: 500, errno: 117`: MessageDataError
+- `code: 500, errno: 117`: Db Error
 - `code: 500, errno: 118`: Not Implemeneted
+- `code: 500, errno: 119`: HMAC error
 
 The following errors include additional response properties:
 

src/app_errors/mod.rs

@@ -150,9 +150,9 @@ pub enum AppErrorKind {
         bounce: BounceRecord,
     },
 
-    /// An error for when an error happens on a request to the db.
+    /// An error occurred inside an auth db method.
     #[fail(display = "{}", _0)]
-    DbError(String),
+    AuthDbError(String),
 
     /// An error for when an error happens on the queues process.
     #[fail(display = "{}", _0)]
@@ -188,14 +188,18 @@ pub enum AppErrorKind {
     #[fail(display = "invalid duration: {}", _0)]
     DurationError(String),
 
-    /// An error for when we get erros in the message_data module.
-    #[fail(display = "{}", _0)]
-    MessageDataError(String),
+    /// An error occured inside a db method.
+    #[fail(display = "Redis error: {}", _0)]
+    DbError(String),
 
     /// An error for when we try to access functionality that is not
     /// implemented.
     #[fail(display = "Feature not implemented")]
     NotImplemented,
+
+    /// An error occured while hashing a value.
+    #[fail(display = "HMAC error: {}", _0)]
+    HmacError(String),
 }
 
 impl AppErrorKind {
@@ -230,7 +234,7 @@ impl AppErrorKind {
             AppErrorKind::BounceSoftError { .. } => Some(107),
             AppErrorKind::BounceHardError { .. } => Some(108),
 
-            AppErrorKind::DbError(_) => Some(109),
+            AppErrorKind::AuthDbError(_) => Some(109),
 
             AppErrorKind::QueueError(_) => Some(110),
             AppErrorKind::InvalidNotificationType => Some(111),
@@ -241,10 +245,12 @@ impl AppErrorKind {
             AppErrorKind::SqsMessageParsingError { .. } => Some(115),
 
             AppErrorKind::DurationError(_) => Some(116),
-            AppErrorKind::MessageDataError(_) => Some(117),
+            AppErrorKind::DbError(_) => Some(117),
 
             AppErrorKind::NotImplemented => Some(118),
 
+            AppErrorKind::HmacError(_) => Some(119),
+
             AppErrorKind::BadRequest
             | AppErrorKind::NotFound
             | AppErrorKind::MethodNotAllowed

src/auth_db/mod.rs

@@ -167,13 +167,13 @@ pub struct BounceRecord {
 
 impl From<UrlError> for AppError {
     fn from(error: UrlError) -> AppError {
-        AppErrorKind::DbError(format!("{}", error)).into()
+        AppErrorKind::AuthDbError(format!("{}", error)).into()
     }
 }
 
 impl From<RequestError> for AppError {
     fn from(error: RequestError) -> AppError {
-        AppErrorKind::DbError(format!("{}", error)).into()
+        AppErrorKind::AuthDbError(format!("{}", error)).into()
     }
 }
 
@@ -237,7 +237,7 @@ impl Db for DbClient {
             .send()?;
         match response.status() {
             StatusCode::Ok => response.json::<Vec<BounceRecord>>().map_err(From::from),
-            status => Err(AppErrorKind::DbError(format!("{}", status)).into()),
+            status => Err(AppErrorKind::AuthDbError(format!("{}", status)).into()),
         }
     }
 
@@ -258,7 +258,7 @@ impl Db for DbClient {
             }).send()?;
         match response.status() {
             StatusCode::Ok => Ok(()),
-            status => Err(AppErrorKind::DbError(format!("{}", status)).into()),
+            status => Err(AppErrorKind::AuthDbError(format!("{}", status)).into()),
         }
     }
 }

src/bounces/test.rs

@@ -281,7 +281,7 @@ pub struct DbMockError;
 
 impl Db for DbMockError {
     fn get_bounces(&self, _address: &str) -> AppResult<Vec<BounceRecord>> {
-        Err(AppErrorKind::DbError(String::from("wibble blee")).into())
+        Err(AppErrorKind::AuthDbError(String::from("wibble blee")).into())
     }
 }
 

src/db.rs

@@ -0,0 +1,113 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, you can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Database abstractions.
+//!
+//! Uses Redis under the hood,
+//! which is fine because
+//! none of the data is relational
+//! and we flip all of the
+//! Redis persistence switches in prod.
+//! You can read more about this decision
+//! in [#166](https://github.com/mozilla/fxa-email-service/issues/166).
+
+use std::fmt::{self, Display, Formatter};
+
+use hmac::{crypto_mac::InvalidKeyLength, Hmac, Mac};
+use redis::{Client as RedisClient, Commands, RedisError};
+use sha2::Sha256;
+
+use app_errors::{AppError, AppErrorKind, AppResult};
+use settings::Settings;
+
+/// Database client.
+///
+/// Really just a thin wrapper
+/// around `redis::Client`,
+/// with some logic for generating keys via HMAC
+/// as a safeguard against leaking PII.
+#[derive(Debug)]
+pub struct Client {
+    client: RedisClient,
+    hmac_key: String,
+}
+
+impl Client {
+    /// Instantiate a db client.
+    pub fn new(settings: &Settings) -> Self {
+        Self {
+            client: RedisClient::open(
+                format!("redis://{}:{}/", settings.redis.host, settings.redis.port).as_str(),
+            ).expect("redis connection error"),
+            hmac_key: settings.hmackey.clone(),
+        }
+    }
+
+    /// Read data.
+    pub fn get(&self, key: &str, data_type: DataType) -> AppResult<String> {
+        let key = self.generate_key(key, data_type)?;
+        self.client.get(key.as_str()).map_err(From::from)
+    }
+
+    /// Read and delete data.
+    pub fn consume(&self, key: &str, data_type: DataType) -> AppResult<String> {
+        let key = self.generate_key(key, data_type)?;
+        let key_str = key.as_str();
+        self.client
+            .get(key_str)
+            .map(|metadata| {
+                self.client.del::<&str, u8>(key_str).ok();
+                metadata
+            }).map_err(From::from)
+    }
+
+    /// Store data.
+    ///
+    /// Any data previously stored for the key
+    /// will be clobbered.
+    pub fn set(&self, key: &str, data: &str, data_type: DataType) -> AppResult<()> {
+        let key = self.generate_key(key, data_type)?;
+        self.client.set(key.as_str(), data).map_err(From::from)
+    }
+
+    fn generate_key(&self, key: &str, data_type: DataType) -> AppResult<String> {
+        let mut hmac: Hmac<Sha256> = Hmac::new_varkey(self.hmac_key.as_bytes())?;
+        hmac.input(key.as_bytes());
+        Ok(format!("{}:{:x}", data_type, hmac.result().code()))
+    }
+}
+
+/// Date types included in this store.
+#[derive(Clone, Copy, Debug)]
+pub enum DataType {
+    Configuration,
+    DeliveryProblem,
+    MessageData,
+}
+
+impl Display for DataType {
+    fn fmt(&self, formatter: &mut Formatter) -> fmt::Result {
+        write!(
+            formatter,
+            "{}",
+            match *self {
+                DataType::Configuration => "cfg",
+                DataType::DeliveryProblem => "del",
+                DataType::MessageData => "msg",
+            }
+        )
+    }
+}
+
+impl From<RedisError> for AppError {
+    fn from(error: RedisError) -> AppError {
+        AppErrorKind::DbError(format!("{:?}", error)).into()
+    }
+}
+
+impl From<InvalidKeyLength> for AppError {
+    fn from(_error: InvalidKeyLength) -> AppError {
+        AppErrorKind::HmacError("invalid key length".to_string()).into()
+    }
+}

src/healthcheck/mod.rs

@@ -2,10 +2,11 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, you can obtain one at https://mozilla.org/MPL/2.0/.
 
-//! Route handlers for our heathcheck endpoints:
-//! for the `GET /__version__` endpoint,
-//! for the `GET /__lbheartbeat__` endpoint and
-//! for the `GET /__heartbeat__` endpoint,
+//! Route handlers for our heathcheck endpoints.
+//!
+//! * `GET /__version__`
+//! * `GET /__lbheartbeat__`
+//! * `GET /__heartbeat__`
 
 use reqwest::Client as RequestClient;
 use rocket::State;
@@ -36,6 +37,6 @@ fn heartbeat(settings: State<Settings>) -> AppResult<Json<JsonValue>> {
 
     match db {
         Ok(_) => Ok(Json(json!({}))),
-        Err(err) => Err(AppErrorKind::DbError(format!("{}", err)).into()),
+        Err(err) => Err(AppErrorKind::AuthDbError(format!("{}", err)).into()),
     }
 }

src/lib.rs

@@ -87,6 +87,7 @@ extern crate uuid;
 pub mod app_errors;
 pub mod auth_db;
 pub mod bounces;
+pub mod db;
 pub mod duration;
 pub mod email_address;
 pub mod healthcheck;

src/message_data/mod.rs

@@ -2,13 +2,10 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, you can obtain one at https://mozilla.org/MPL/2.0/.
 
-//! Temporary storage for message metadata.
+//! Storage for message metadata.
 
-use hmac::{crypto_mac::InvalidKeyLength, Hmac, Mac};
-use redis::{Client as RedisClient, Commands, RedisError};
-use sha2::Sha256;
-
-use app_errors::{AppError, AppErrorKind, AppResult};
+use app_errors::AppResult;
+use db::{Client as DbClient, DataType};
 use settings::Settings;
 
 #[cfg(test)]
@@ -16,27 +13,17 @@ mod test;
 
 /// Message data store.
 ///
-/// Currently uses Redis
-/// under the hood,
-/// although that may not
-/// always be the case.
-///
-/// Data is keyed by
-/// a hash of the message id.
+/// Data is keyed by message id.
 #[derive(Debug)]
 pub struct MessageData {
-    client: RedisClient,
-    hmac_key: String,
+    client: DbClient,
 }
 
 impl MessageData {
     /// Instantiate a storage client.
-    pub fn new(settings: &Settings) -> MessageData {
-        MessageData {
-            client: RedisClient::open(
-                format!("redis://{}:{}/", settings.redis.host, settings.redis.port).as_str(),
-            ).expect("redis connection error"),
-            hmac_key: settings.hmackey.clone(),
+    pub fn new(settings: &Settings) -> Self {
+        Self {
+            client: DbClient::new(settings),
         }
     }
 
@@ -46,14 +33,7 @@ impl MessageData {
     /// Once consumed,
     /// the data is permanently destroyed.
     pub fn consume(&self, message_id: &str) -> AppResult<String> {
-        let key = self.generate_key(message_id)?;
-        let key_str = key.as_str();
-        self.client
-            .get(key_str)
-            .map(|metadata| {
-                self.client.del::<&str, u8>(key_str).ok();
-                metadata
-            }).map_err(From::from)
+        self.client.consume(message_id, DataType::MessageData)
     }
 
     /// Store message metadata.
@@ -61,25 +41,6 @@ impl MessageData {
     /// Any data previously stored for the message id
     /// will be replaced.
     pub fn set(&self, message_id: &str, metadata: &str) -> AppResult<()> {
-        let key = self.generate_key(message_id)?;
-        self.client.set(key.as_str(), metadata).map_err(From::from)
-    }
-
-    fn generate_key(&self, message_id: &str) -> AppResult<String> {
-        let mut hmac = Hmac::<Sha256>::new_varkey(self.hmac_key.as_bytes())?;
-        hmac.input(message_id.as_bytes());
-        Ok(format!("msg:{:x}", hmac.result().code()))
-    }
-}
-
-impl From<RedisError> for AppError {
-    fn from(error: RedisError) -> AppError {
-        AppErrorKind::MessageDataError(format!("redis error: {:?}", error)).into()
-    }
-}
-
-impl From<InvalidKeyLength> for AppError {
-    fn from(error: InvalidKeyLength) -> AppError {
-        AppErrorKind::MessageDataError(format!("hmac key error: {:?}", error)).into()
+        self.client.set(message_id, metadata, DataType::MessageData)
     }
 }

src/message_data/test.rs

@@ -55,7 +55,7 @@ fn consume() {
     );
     match test.message_data.consume(&test.unhashed_key) {
         Ok(_) => assert!(false, "consume should fail when called a second time"),
-        Err(error) => assert_eq!(format!("{}", error), "redis error: Response was of incompatible type: \"Response type not string compatible.\" (response was nil)"),
+        Err(error) => assert_eq!(format!("{}", error), "Redis error: Response was of incompatible type: \"Response type not string compatible.\" (response was nil)"),
     }
 }