Merge pull request #918 from cloudflare/alewis/refactor-routes

Alewis/refactor routes

Cargo.toml

@@ -17,7 +17,7 @@ clap = "2.32.0"
 config = "0.9.2"
 console = "0.7.5"
 dirs = "1.0.5"
-cloudflare = "0.4.1"
+cloudflare = { path = "/Users/ashleylewis/cloudflare-rs" }
 env_logger = "0.6.1"
 failure = "0.1.5"
 log = "0.4.6"

src/commands/publish/mod.rs

@@ -4,7 +4,7 @@ mod route;
 pub mod upload_form;
 
 pub use package::Package;
-use route::{publish_route, Route};
+use route::publish_route;
 
 use std::env;
 use std::path::Path;
@@ -101,17 +101,15 @@ fn publish_script(
         .send()?;
 
     let res_status = res.status();
-    let res_text = res.text()?;
 
     if !res_status.is_success() {
+        let res_text = res.text()?;
         failure::bail!(error_msg(res_status, res_text))
     }
 
     let pattern = if target.route.is_some() {
-        let route = Route::new(&target)?;
-        publish_route(&user, &target, &route)?;
         log::info!("publishing to route");
-        route.pattern
+        publish_route(&user, &target)?
     } else {
         log::info!("publishing to subdomain");
         publish_to_subdomain(target, user)?

src/commands/publish/route.rs

@@ -1,64 +1,23 @@
-use serde::{Deserialize, Serialize};
-
-use cloudflare::endpoints::workers::{CreateRoute, CreateRouteParams, ListRoutes, WorkersRoute};
+use cloudflare::endpoints::workers::{CreateRoute, CreateRouteParams, ListRoutes};
 use cloudflare::framework::apiclient::ApiClient;
 use cloudflare::framework::HttpApiClientConfig;
 
 use crate::http::{api_client, format_error};
 use crate::settings::global_user::GlobalUser;
-use crate::settings::target::Target;
-use crate::terminal::emoji;
-
-#[derive(Deserialize, PartialEq, Serialize)]
-pub struct Route {
-    script: Option<String>,
-    pub pattern: String,
-}
-
-impl From<&WorkersRoute> for Route {
-    fn from(api_route: &WorkersRoute) -> Route {
-        Route {
-            script: api_route.script.clone(),
-            pattern: api_route.pattern.clone(),
-        }
-    }
-}
-
-impl Route {
-    pub fn new(target: &Target) -> Result<Route, failure::Error> {
-        if target
-            .route
-            .clone()
-            .expect("You must provide a zone_id in your wrangler.toml before publishing!")
-            .is_empty()
-        {
-            failure::bail!("You must provide a zone_id in your wrangler.toml before publishing!");
-        }
-        let msg_config_error = format!(
-            "{} Your project config has an error, check your `wrangler.toml`: `route` must be provided.", 
-            emoji::WARN
-        );
-        Ok(Route {
-            script: Some(target.name.to_string()),
-            pattern: target.route.clone().expect(&msg_config_error),
-        })
-    }
-}
+use crate::settings::target::{Route, Target};
 
-pub fn publish_route(
-    user: &GlobalUser,
-    target: &Target,
-    route: &Route,
-) -> Result<(), failure::Error> {
-    if route_exists(user, target, route)? {
-        Ok(())
+pub fn publish_route(user: &GlobalUser, target: &Target) -> Result<String, failure::Error> {
+    let route = Route::new(&target)?;
+    if route_exists(user, target, &route)? {
+        Ok(route.pattern)
     } else {
-        create(user, target, route)
+        create(user, target, &route)?;
+        Ok(route.pattern)
     }
 }
 
 fn route_exists(user: &GlobalUser, target: &Target, route: &Route) -> Result<bool, failure::Error> {
-    let routes = get_routes(user, target)?;
+    let routes = fetch_all(user, target)?;
 
     for remote_route in routes {
         if remote_route == *route {
@@ -68,7 +27,7 @@ fn route_exists(user: &GlobalUser, target: &Target, route: &Route) -> Result<boo
     Ok(false)
 }
 
-fn get_routes(user: &GlobalUser, target: &Target) -> Result<Vec<Route>, failure::Error> {
+fn fetch_all(user: &GlobalUser, target: &Target) -> Result<Vec<Route>, failure::Error> {
     let client = api_client(user, HttpApiClientConfig::default())?;
 
     let routes: Vec<Route> = match client.request(&ListRoutes {

src/settings/target/environment.rs

@@ -1,8 +1,6 @@
 use super::kv_namespace::KvNamespace;
 use super::site::Site;
 
-use std::collections::HashMap;
-
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
@@ -11,7 +9,7 @@ pub struct Environment {
     pub account_id: Option<String>,
     pub workers_dev: Option<bool>,
     pub route: Option<String>,
-    pub routes: Option<HashMap<String, String>>,
+    pub routes: Option<Vec<String>>,
     pub zone_id: Option<String>,
     pub webpack_config: Option<String>,
     pub private: Option<bool>,

src/settings/target/manifest.rs

@@ -31,7 +31,7 @@ pub struct Manifest {
     pub workers_dev: Option<bool>,
     #[serde(default = "some_string")]
     pub route: Option<String>,
-    pub routes: Option<HashMap<String, String>>,
+    pub routes: Option<Vec<String>>,
     #[serde(default = "some_string")]
     pub zone_id: Option<String>,
     pub webpack_config: Option<String>,

src/settings/target/mod.rs

@@ -1,13 +1,15 @@
 mod environment;
 mod kv_namespace;
 mod manifest;
+mod route;
 mod site;
 mod target;
 mod target_type;
 
 pub use environment::Environment;
 pub use kv_namespace::KvNamespace;
 pub use manifest::Manifest;
+pub use route::Route;
 pub use site::Site;
 pub use target::Target;
 pub use target_type::TargetType;

src/settings/target/route.rs

@@ -0,0 +1,42 @@
+use serde::{Deserialize, Serialize};
+
+use cloudflare::endpoints::workers::WorkersRoute;
+
+use crate::settings::target::target::Target;
+use crate::terminal::emoji;
+
+#[derive(Deserialize, PartialEq, Serialize)]
+pub struct Route {
+    pub script: Option<String>,
+    pub pattern: String,
+}
+
+impl From<&WorkersRoute> for Route {
+    fn from(api_route: &WorkersRoute) -> Route {
+        Route {
+            script: api_route.script.clone(),
+            pattern: api_route.pattern.clone(),
+        }
+    }
+}
+
+impl Route {
+    pub fn new(target: &Target) -> Result<Route, failure::Error> {
+        if target
+            .route
+            .clone()
+            .expect("You must provide a zone_id in your wrangler.toml before publishing!")
+            .is_empty()
+        {
+            failure::bail!("You must provide a zone_id in your wrangler.toml before publishing!");
+        }
+        let msg_config_error = format!(
+            "{} Your project config has an error, check your `wrangler.toml`: `route` must be provided.", 
+            emoji::WARN
+        );
+        Ok(Route {
+            script: Some(target.name.to_string()),
+            pattern: target.route.clone().expect(&msg_config_error),
+        })
+    }
+}

src/settings/target/target.rs

@@ -1,9 +1,8 @@
 use super::kv_namespace::KvNamespace;
-
 use super::site::Site;
 use super::target_type::TargetType;
+use super::Route;
 
-use std::collections::HashMap;
 use std::env;
 
 use std::path::PathBuf;
@@ -19,7 +18,7 @@ pub struct Target {
     #[serde(rename = "type")]
     pub target_type: TargetType,
     pub route: Option<String>,
-    pub routes: Option<HashMap<String, String>>,
+    pub routes: Option<Vec<String>>,
     pub webpack_config: Option<String>,
     pub zone_id: Option<String>,
     pub site: Option<Site>,
@@ -47,4 +46,31 @@ impl Target {
             }
         }
     }
+
+    pub fn routes(&self) -> Result<Vec<Route>, failure::Error> {
+        let mut routes = Vec::new();
+
+        // we should assert that only one of the two keys is specified in the user's toml.
+        if self.route.is_some() && self.routes.is_some() {
+            failure::bail!("You can specify EITHER `route` or `routes` in your wrangler.toml");
+        }
+
+        // everything outside of this module should consider `target.routes()` to be a Vec;
+        // the fact that you can specify singular or plural is a detail of the wrangler.toml contract.
+        if let Some(single_route) = &self.route {
+            routes.push(Route {
+                script: Some(self.name.to_owned()),
+                pattern: single_route.to_string(),
+            });
+        } else if let Some(multi_route) = &self.routes {
+            for pattern in multi_route {
+                routes.push(Route {
+                    script: Some(self.name.to_owned()),
+                    pattern: pattern.to_string(),
+                });
+            }
+        }
+
+        Ok(routes)
+    }
 }