refactor(settings): group provider-related settings together

mozilla · Jul 30, 2018 · c115318 · c115318
1 parent ca4b201
commit c115318
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 44 deletions.
diff --git a/config/default.json b/config/default.json
@@ -26,8 +26,10 @@
     "format": "mozlog"
   },
   "port": 8001,
-  "provider": "ses",
-  "forceprovider": false,
+  "provider": {
+    "default": "ses",
+    "forcedefault": false
+  },
   "redis": {
     "host": "127.0.0.1",
     "port": 6379

diff --git a/config/dev.json b/config/dev.json
@@ -3,8 +3,10 @@
     "level": "normal",
     "format": "pretty"
   },
-  "provider": "smtp",
-  "forceprovider": true,
+  "provider": {
+    "default": "smtp",
+    "forcedefault": true
+  },
   "smtp": {
     "port": 9999
   }

diff --git a/src/providers/mod.rs b/src/providers/mod.rs
@@ -13,7 +13,7 @@ use self::{
     smtp::SmtpProvider as Smtp, socketlabs::SocketLabsProvider as SocketLabs,
 };
 use app_errors::{AppErrorKind, AppResult};
-use settings::{Provider as SettingsProvider, Settings};
+use settings::{DefaultProvider, Settings};
 
 mod mock;
 mod sendgrid;
@@ -106,8 +106,8 @@ impl Providers {
 
         macro_rules! set_provider {
             ($id:expr, $constructor:expr) => {
-                if !settings.forceprovider
-                    || settings.provider == SettingsProvider(String::from($id))
+                if !settings.provider.forcedefault
+                    || settings.provider.default == DefaultProvider(String::from($id))
                 {
                     providers.insert(String::from($id), Box::new($constructor));
                 }
@@ -127,8 +127,8 @@ impl Providers {
         }
 
         Providers {
-            default_provider: settings.provider.0.clone(),
-            force_default_provider: settings.forceprovider,
+            default_provider: settings.provider.default.0.clone(),
+            force_default_provider: settings.provider.forcedefault,
             providers,
         }
     }

diff --git a/src/providers/test.rs b/src/providers/test.rs
@@ -105,13 +105,13 @@ fn build_mime_with_body_html() {
 #[test]
 fn constructor() {
     let mut settings = Settings::new().expect("config error");
-    settings.forceprovider = false;
+    settings.provider.forcedefault = false;
     let providers = Providers::new(&settings);
     assert!(providers.providers.len() > 1);
     assert_eq!(providers.force_default_provider, false);
 
     settings = Settings::new().expect("config error");
-    settings.forceprovider = true;
+    settings.provider.forcedefault = true;
     let providers = Providers::new(&settings);
     assert_eq!(providers.providers.len(), 1);
     assert_eq!(providers.force_default_provider, true);
@@ -120,17 +120,17 @@ fn constructor() {
 #[test]
 fn send() {
     let mut settings = Settings::new().expect("config error");
-    settings.forceprovider = true;
-    settings.provider = SettingsProvider(String::from("mock"));
+    settings.provider.forcedefault = true;
+    settings.provider.default = DefaultProvider(String::from("mock"));
     let providers = Providers::new(&settings);
     let result = providers.send("foo", &vec![], None, "bar", "baz", None, Some("ses"));
     assert!(result.is_ok(), "Providers::send should not have failed");
     if let Ok(ref message_id) = result {
         assert_eq!(message_id, "mock:deadbeef");
     }
 
-    settings.forceprovider = false;
-    settings.provider = SettingsProvider(String::from("ses"));
+    settings.provider.forcedefault = false;
+    settings.provider.default = DefaultProvider(String::from("ses"));
     let providers = Providers::new(&settings);
     let result = providers.send("foo", &vec![], None, "bar", "baz", None, Some("mock"));
     assert!(result.is_ok(), "Providers::send should not have failed");

diff --git a/src/send/test.rs b/src/send/test.rs
@@ -18,7 +18,7 @@ use settings::Settings;
 
 fn setup() -> Client {
     let mut settings = Settings::new().unwrap();
-    settings.forceprovider = false;
+    settings.provider.forcedefault = false;
     let db = DbClient::new(&settings);
     let bounces = Bounces::new(&settings, db);
     let logger = MozlogLogger::new(&settings).expect("MozlogLogger::init error");

diff --git a/src/settings/mod.rs b/src/settings/mod.rs
@@ -63,6 +63,8 @@ deserialize_and_validate! {
     (AwsSecret, aws_secret, "AWS secret key"),
     /// Base URI type.
     (BaseUri, base_uri, "base URI"),
+    /// Default email provider.
+    (DefaultProvider, provider, "'ses', 'sendgrid', 'socketlabs' or 'smtp'"),
     /// Env type.
     (Env, env, "'dev', 'staging', 'production' or 'test'"),
     /// Host name or IP address type.
@@ -71,8 +73,6 @@ deserialize_and_validate! {
     (LoggingLevel, logging_level, "'normal', 'debug', 'critical' or 'off'"),
     /// Logging format type.
     (LoggingFormat, logging_format, "'mozlog', 'pretty' or 'null'"),
-    /// Email provider type.
-    (Provider, provider, "'ses', 'sendgrid' or 'smtp'"),
     /// Sender name type.
     (SenderName, sender_name, "sender name"),
     /// Sendgrid API key type.
@@ -158,6 +158,21 @@ pub struct Log {
     pub format: LoggingFormat,
 }
 
+/// Email provider settings.
+#[derive(Debug, Default, Deserialize, Serialize)]
+pub struct Provider {
+    /// The default email provider to use,
+    /// can be `"ses"`, `"sendgrid"`, `"socketlabs"`, `"smtp"` or `"mock"`.
+    /// Note that this setting can be overridden
+    /// on a per-request basis
+    /// unless `forcedefault` is `true`.
+    pub default: DefaultProvider,
+
+    /// Flag indicating whether the default provider should be enforced
+    /// in preference to the per-request `provider` param.
+    pub forcedefault: bool,
+}
+
 /// Settings for Redis.
 #[derive(Debug, Default, Deserialize, Serialize)]
 pub struct Redis {
@@ -284,13 +299,6 @@ pub struct Settings {
     /// It defaults to `dev` if not set.
     pub env: Env,
 
-    /// Flag indicating whether
-    /// the default `provider` from config
-    /// should be enforced
-    /// in preference to
-    /// the per-request `provider` param.
-    pub forceprovider: bool,
-
     /// The HMAC key to use internally
     /// for hashing message ids.
     /// This is sensitive data
@@ -306,10 +314,7 @@ pub struct Settings {
     /// The port this application is listening to.
     pub port: u16,
 
-    /// The default email provider to use,
-    /// can be `"ses"`, `"sendgrid"` or `"mock"`.
-    /// Note that this setting can be overridden
-    /// on a per-request basis.
+    /// Settings controlling the default email provider.
     pub provider: Provider,
 
     /// Settings for Redis,

diff --git a/src/settings/test.rs b/src/settings/test.rs
@@ -129,12 +129,16 @@ fn env_vars_take_precedence() {
             let bounce_limits_enabled = !settings.bouncelimits.enabled;
             let current_env = Env(String::from("test"));
             let hmac_key = String::from("something else");
-            let provider = if settings.provider == Provider("ses".to_string()) {
-                "sendgrid"
-            } else {
-                "ses"
+            let provider = Provider {
+                default: DefaultProvider(
+                    if settings.provider.default == DefaultProvider("ses".to_string()) {
+                        "sendgrid"
+                    } else {
+                        "ses"
+                    }.to_string(),
+                ),
+                forcedefault: !settings.provider.forcedefault,
             };
-            let forceprovider = !settings.forceprovider;
             let redis_host = format!("{}1", &settings.redis.host);
             let redis_port = settings.redis.port + 1;
             let secretkey = String::from("ampqampqampqampqampqampqampqampqampqampqamo=");
@@ -201,8 +205,11 @@ fn env_vars_take_precedence() {
             env::set_var("FXA_EMAIL_ENV", &current_env.0);
             env::set_var("FXA_EMAIL_LOG_LEVEL", &log.level.0);
             env::set_var("FXA_EMAIL_LOG_FORMAT", &log.format.0);
-            env::set_var("FXA_EMAIL_PROVIDER", &provider);
-            env::set_var("FXA_EMAIL_FORCEPROVIDER", forceprovider.to_string());
+            env::set_var("FXA_EMAIL_PROVIDER_DEFAULT", &provider.default.0);
+            env::set_var(
+                "FXA_EMAIL_PROVIDER_FORCEDEFAULT",
+                provider.forcedefault.to_string(),
+            );
             env::set_var("FXA_EMAIL_REDIS_HOST", &redis_host);
             env::set_var("FXA_EMAIL_REDIS_PORT", &redis_port.to_string());
             env::set_var("FXA_EMAIL_SECRETKEY", &secretkey);
@@ -231,8 +238,8 @@ fn env_vars_take_precedence() {
                     assert_eq!(env_settings.hmackey, hmac_key);
                     assert_eq!(env_settings.log.level, log.level);
                     assert_eq!(env_settings.log.format, log.format);
-                    assert_eq!(env_settings.provider, Provider(provider.to_string()));
-                    assert_eq!(env_settings.forceprovider, forceprovider);
+                    assert_eq!(env_settings.provider.default, provider.default);
+                    assert_eq!(env_settings.provider.forcedefault, provider.forcedefault);
                     assert_eq!(env_settings.redis.host, Host(redis_host));
                     assert_eq!(env_settings.redis.port, redis_port);
                     assert_eq!(env_settings.secretkey, secretkey);
@@ -471,9 +478,9 @@ fn invalid_smtp_host() {
 }
 
 #[test]
-fn invalid_provider() {
-    let _clean_env = CleanEnvironment::new(vec!["FXA_EMAIL_PROVIDER"]);
-    env::set_var("FXA_EMAIL_PROVIDER", "sess");
+fn invalid_provider_default() {
+    let _clean_env = CleanEnvironment::new(vec!["FXA_EMAIL_PROVIDER_DEFAULT"]);
+    env::set_var("FXA_EMAIL_PROVIDER_DEFAULT", "sess");
 
     match Settings::new() {
         Ok(_settings) => assert!(false, "Settings::new should have failed"),
@@ -482,9 +489,9 @@ fn invalid_provider() {
 }
 
 #[test]
-fn invalid_forceprovider() {
-    let _clean_env = CleanEnvironment::new(vec!["FXA_EMAIL_FORCEPROVIDER"]);
-    env::set_var("FXA_EMAIL_FORCEPROVIDER", "wibble");
+fn invalid_provider_forcedefault() {
+    let _clean_env = CleanEnvironment::new(vec!["FXA_EMAIL_PROVIDER_FORCEDEFAULT"]);
+    env::set_var("FXA_EMAIL_PROVIDER_FORCEDEFAULT", "wibble");
 
     match Settings::new() {
         Ok(_settings) => assert!(false, "Settings::new should have failed"),