Don't require an account_id in wrangler.toml (#1966)

* Switch from lazy_static to once_cell

`OnceCell` is needed later, and this avoids having two dependencies that
do the same thing. Wrangler has dependencies that use both, so this
doesn't actually reduce compile times, but it will make it clear which
library to use.

* Use the None variant of `RouteId::account_id`

Previously, it was always `Some` and the empty string was used to see if
it wasn't filled out. This removes unnecessary checking for the empty
string.

* Use an Option for account_id

This makes it explicit where the account ID is required and where it's
treated as the empty string.

* Introduce LazyAccountId

This collects all handling of the account_id in one place, so that
errors no longer have to be handled at each call-site.

Making this a new type, rather than a function on `Manifest`, allows it
to be used for other types as well, such as `Target`.

Using a `OnceCell` rather than an `Option` has two benefits:
1. It does not allow setting the account twice, which would be a bug.
2. It allows setting the account with only an immutable reference, which
   reduces the amount of churn in calling functions.

Note that this still deserializes the empty string as not having an `account_id`
for backwards compatibility.

* Handle multiple accounts

* Load account lazily in Target too

This avoid the following test failure:

```
it_builds_with_webpack_target_webworker_with_custom_file stdout ----
Created fixture at /tmp/.tmpSzLIJb
thread 'it_builds_with_webpack_target_webworker_with_custom_file' panicked at 'Build failed: Output { status: ExitStatus(ExitStatus(256)), stdout: "👀  You have multiple accounts.\nPlease choose one from below and add it to `wrangler.toml` under `account_id`.\n🕵\u{fe0f}  You can copy your account_id below\n+--------------------------+----------------------------------+\n| Account Name             | Account ID                       |\n+--------------------------+----------------------------------+\n| Cloudflare Community Jam | 6174b0f52802f24a9f52e7015282898c |\n+--------------------------+----------------------------------+\n| Edge Workers             | 615f1f0479e7014f0bebcd10d379f10e |\n+--------------------------+----------------------------------+\n", stderr: "Error: \n" }', tests/build.rs:377:5
```

It also prevents any similar issues that aren't tested, by forcing each
command that wants an account ID to load it explicitly.

* Fix previews

Previews have different behavior when an account is available than when
it isn't. This restores that previous behavior: if an account is
available, use authenticated previews, otherwise unauthenticated
previews.

Note that "an account is available" has become a slightly tricky
question, since it's no longer guaranteed that it will be in the
wrangler.toml. Instead, this checks if it is either in the .toml or if
there is only a single account available to the authentication token.

* Use `maybe_load` instead of `if_present` for `load_project_info`

This only affects `wrangler report`. The old behavior preserved the
behavior from before, but was less helpful because it wouldn't include
the ID if there was only a single ID for the account token.

* Remove unneeded `validate`  function

It no longer does anything now that the account ID is loaded on demand.

* Don't tell the user to add `account_id` after running `wrangler generate`

Before, wrangler would give this message:
```
> wrangler generate
  Creating project called `worker`...
  Done! New project created /home/jnelson/src/test-project/worker/worker
  🕵️  You can find your zone_id in the right sidebar of a zone's overview tab at https://dash.cloudflare.com
  🕵️  You can copy your account_id below
  +----------------------------+----------------------------------+
  | Account Name               | Account ID                       |
  +----------------------------+----------------------------------+
  | Jyn514@gmail.com's Account | e1706d218241c1230155b4f91b3218af |
  +----------------------------+----------------------------------+
  🕵️  You will need to update the following fields in the created wrangler.toml file before continuing:
  - account_id
```

Adding the account ID is no longer necessary, so don't suggest adding it.

Cargo.toml

@@ -35,10 +35,10 @@ hyper = { version = "0.14.7", features = ["http2", "server", "runtime"] }
 hyper-rustls = "0.22.1"
 ignore = "0.4.17"
 indicatif = "0.15.0"
-lazy_static = "1.4.0"
 log = "0.4.11"
 notify = "4.0.15"
 number_prefix = "0.4.0"
+once_cell = "1"
 openssl = { version = "0.10.32", optional = true }
 os-version = "0.1.1"
 path-slash = "0.1.4"

src/commands/dev/edge/setup.rs

@@ -41,7 +41,7 @@ pub(super) fn upload(
     };
 
     let session_config = get_session_config(deploy_target);
-    let address = get_upload_address(target);
+    let address = get_upload_address(target)?;
 
     let script_upload_form = upload::form::build(target, asset_manifest, Some(session_config))?;
 
@@ -144,11 +144,12 @@ fn get_session_address(target: &DeployTarget) -> String {
     }
 }
 
-fn get_upload_address(target: &mut Target) -> String {
-    format!(
+fn get_upload_address(target: &mut Target) -> Result<String> {
+    Ok(format!(
         "https://api.cloudflare.com/client/v4/accounts/{}/workers/scripts/{}/edge-preview",
-        target.account_id, target.name
-    )
+        target.account_id.load()?,
+        target.name
+    ))
 }
 
 fn get_exchange_url(deploy_target: &DeployTarget, user: &GlobalUser) -> Result<Url> {

src/commands/kv/bulk/delete.rs

@@ -9,16 +9,14 @@ use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair;
 use anyhow::Result;
 use indicatif::{ProgressBar, ProgressStyle};
 
-use crate::commands::kv;
 use crate::kv::bulk::delete;
 use crate::kv::bulk::BATCH_KEY_MAX;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::interactive;
 use crate::terminal::message::{Message, StdOut};
-pub fn run(target: &Target, user: &GlobalUser, namespace_id: &str, filename: &Path) -> Result<()> {
-    kv::validate_target(target)?;
 
+pub fn run(target: &Target, user: &GlobalUser, namespace_id: &str, filename: &Path) -> Result<()> {
     match interactive::confirm(&format!(
         "Are you sure you want to delete all keys in {}?",
         filename.display()

src/commands/kv/bulk/put.rs

@@ -9,15 +9,12 @@ use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair;
 use anyhow::{anyhow, Result};
 use indicatif::{ProgressBar, ProgressStyle};
 
-use crate::commands::kv::validate_target;
 use crate::kv::bulk::put;
 use crate::kv::bulk::BATCH_KEY_MAX;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::message::{Message, StdErr};
 pub fn run(target: &Target, user: &GlobalUser, namespace_id: &str, filename: &Path) -> Result<()> {
-    validate_target(target)?;
-
     let pairs: Vec<KeyValuePair> = match &metadata(filename) {
         Ok(file_type) if file_type.is_file() => {
             let data = fs::read_to_string(filename)?;

src/commands/kv/key/delete.rs

@@ -3,14 +3,13 @@ use cloudflare::framework::apiclient::ApiClient;
 
 use anyhow::Result;
 
-use crate::commands::kv::{format_error, validate_target};
+use crate::commands::kv::format_error;
 use crate::http;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
 use crate::terminal::interactive;
 use crate::terminal::message::{Message, StdOut};
 pub fn delete(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result<()> {
-    validate_target(target)?;
     let client = http::cf_v4_client(user)?;
 
     match interactive::confirm(&format!("Are you sure you want to delete key \"{}\"?", key)) {
@@ -26,7 +25,7 @@ pub fn delete(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result
     StdOut::working(&msg);
 
     let response = client.request(&DeleteKey {
-        account_identifier: &target.account_id,
+        account_identifier: target.account_id.load()?,
         namespace_identifier: id,
         key, // this is url encoded within cloudflare-rs
     });

src/commands/kv/key/get.rs

@@ -14,10 +14,9 @@ use crate::settings::toml::Target;
 use std::io::{self, Write};
 
 pub fn get(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result<()> {
-    kv::validate_target(target)?;
     let api_endpoint = format!(
         "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}",
-        target.account_id,
+        target.account_id.load()?,
         id,
         kv::url_encode_key(key)
     );

src/commands/kv/key/list.rs

@@ -17,7 +17,6 @@ pub fn list(
     namespace_id: &str,
     prefix: Option<&str>,
 ) -> Result<()> {
-    kv::validate_target(target)?;
     let client = http::cf_v4_client(&user)?;
     let key_list = KeyList::new(target, client, namespace_id, prefix)?;
 

src/commands/kv/key/put.rs

@@ -49,11 +49,9 @@ pub fn parse_metadata(arg: Option<&str>) -> Result<Option<serde_json::Value>> {
 }
 
 pub fn put(target: &Target, user: &GlobalUser, data: KVMetaData) -> Result<()> {
-    kv::validate_target(target)?;
-
     let api_endpoint = format!(
         "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}",
-        target.account_id,
+        target.account_id.load()?,
         &data.namespace_id,
         kv::url_encode_key(&data.key)
     );

src/commands/kv/mod.rs

@@ -39,23 +39,6 @@ fn kv_help(error_code: u16) -> &'static str {
     }
 }
 
-pub fn validate_target(target: &Target) -> Result<()> {
-    let mut missing_fields = Vec::new();
-
-    if target.account_id.is_empty() {
-        missing_fields.push("account_id")
-    };
-
-    if !missing_fields.is_empty() {
-        anyhow::bail!(
-            "Your configuration file is missing the following field(s): {:?}",
-            missing_fields
-        )
-    } else {
-        Ok(())
-    }
-}
-
 fn check_duplicate_namespaces(target: &Target) -> bool {
     // HashSet for detecting duplicate namespace bindings
     let mut binding_names: HashSet<String> = HashSet::new();
@@ -109,7 +92,7 @@ mod tests {
     #[test]
     fn it_can_detect_duplicate_bindings() {
         let target_with_dup_kv_bindings = Target {
-            account_id: "".to_string(),
+            account_id: None.into(),
             kv_namespaces: vec![
                 KvNamespace {
                     id: "fake".to_string(),

src/commands/kv/namespace/delete.rs

@@ -9,7 +9,6 @@ use crate::terminal::message::{Message, StdOut};
 use anyhow::Result;
 
 pub fn run(target: &Target, user: &GlobalUser, id: &str) -> Result<()> {
-    kv::validate_target(target)?;
     let client = http::cf_v4_client(user)?;
 
     match interactive::confirm(&format!(
@@ -27,7 +26,7 @@ pub fn run(target: &Target, user: &GlobalUser, id: &str) -> Result<()> {
     let msg = format!("Deleting namespace {}", id);
     StdOut::working(&msg);
 
-    let response = delete(client, target, id);
+    let response = delete(client, target.account_id.load()?, id);
     match response {
         Ok(_) => {
             StdOut::success("Success");

src/commands/kv/namespace/list.rs

@@ -1,4 +1,3 @@
-use crate::commands::kv;
 use crate::http;
 use crate::kv::namespace::list;
 use crate::settings::global_user::GlobalUser;
@@ -7,8 +6,6 @@ use crate::settings::toml::Target;
 use anyhow::Result;
 
 pub fn run(target: &Target, user: &GlobalUser) -> Result<()> {
-    kv::validate_target(target)?;
-
     let client = http::cf_v4_client(user)?;
     let result = list(&client, target);
     match result {

src/commands/publish.rs

@@ -184,9 +184,6 @@ pub fn validate_bucket_location(bucket: &Path) -> Result<()> {
 fn validate_target_required_fields_present(target: &Target) -> Result<()> {
     let mut missing_fields = Vec::new();
 
-    if target.account_id.is_empty() {
-        missing_fields.push("account_id")
-    };
     if target.name.is_empty() {
         missing_fields.push("name")
     };

src/commands/secret.rs

@@ -7,32 +7,14 @@ use anyhow::Result;
 use crate::http;
 use crate::settings::global_user::GlobalUser;
 use crate::settings::toml::Target;
+use crate::terminal::interactive;
 use crate::terminal::message::{Message, StdOut};
-use crate::terminal::{emoji, interactive};
 use crate::upload;
 
 fn format_error(e: ApiFailure) -> String {
     http::format_error(e, Some(&secret_errors))
 }
 
-fn validate_target(target: &Target) -> Result<()> {
-    let mut missing_fields = Vec::new();
-
-    if target.account_id.is_empty() {
-        missing_fields.push("account_id")
-    };
-
-    if !missing_fields.is_empty() {
-        anyhow::bail!(
-            "{} Your configuration file is missing the following field(s): {:?}",
-            emoji::WARN,
-            missing_fields
-        )
-    } else {
-        Ok(())
-    }
-}
-
 // secret_errors() provides more detailed explanations of API error codes.
 fn secret_errors(error_code: u16) -> &'static str {
     match error_code {
@@ -71,8 +53,6 @@ pub fn upload_draft_worker(
 }
 
 pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<()> {
-    validate_target(target)?;
-
     let secret_value = interactive::get_user_input_multi_line(&format!(
         "Enter the secret text you'd like assigned to the variable {} on the script named {}:",
         name, target.name
@@ -96,7 +76,7 @@ pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<(
     };
 
     let response = client.request(&CreateSecret {
-        account_identifier: &target.account_id,
+        account_identifier: target.account_id.load()?,
         script_name: &target.name,
         params: params.clone(),
     });
@@ -108,7 +88,7 @@ pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<(
             Some(draft_upload_response) => match draft_upload_response {
                 Ok(_) => {
                     let retry_response = client.request(&CreateSecret {
-                        account_identifier: &target.account_id,
+                        account_identifier: target.account_id.load()?,
                         script_name: &target.name,
                         params,
                     });
@@ -127,8 +107,6 @@ pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<(
 }
 
 pub fn delete_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<()> {
-    validate_target(target)?;
-
     match interactive::confirm(&format!(
         "Are you sure you want to permanently delete the variable {} on the script named {}?",
         name, target.name
@@ -149,7 +127,7 @@ pub fn delete_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<(
     let client = http::cf_v4_client(user)?;
 
     let response = client.request(&DeleteSecret {
-        account_identifier: &target.account_id,
+        account_identifier: target.account_id.load()?,
         script_name: &target.name,
         secret_name: &name,
     });
@@ -163,11 +141,10 @@ pub fn delete_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<(
 }
 
 pub fn list_secrets(user: &GlobalUser, target: &Target) -> Result<()> {
-    validate_target(target)?;
     let client = http::cf_v4_client(user)?;
 
     let response = client.request(&ListSecrets {
-        account_identifier: &target.account_id,
+        account_identifier: target.account_id.load()?,
         script_name: &target.name,
     });
 

src/commands/subdomain.rs

@@ -114,25 +114,20 @@ fn register_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result<
         name
     );
     StdOut::working(&msg);
-    Subdomain::put(name, &target.account_id, user)
+    Subdomain::put(name, target.account_id.load()?, user)
 }
 
 pub fn set_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result<()> {
-    if target.account_id.is_empty() {
-        anyhow::bail!(
-            "{} You must provide an account_id in your configuration file before creating a subdomain!",
-            emoji::WARN
-        )
-    }
-    let subdomain = Subdomain::get(&target.account_id, user)?;
+    let account_id = target.account_id.load()?;
+    let subdomain = Subdomain::get(account_id, user)?;
     if let Some(subdomain) = subdomain {
         if subdomain == name {
             let msg = format!("You have already registered {}.workers.dev", subdomain);
             StdOut::success(&msg);
             return Ok(());
         } else {
             // list all the affected scripts
-            let scripts = get_subdomain_scripts(&target.account_id, user)?;
+            let scripts = get_subdomain_scripts(account_id, user)?;
 
             let default_msg = format!("Are you sure you want to permanently move your subdomain from {}.workers.dev to {}.workers.dev?",
                                       subdomain, name);
@@ -168,7 +163,7 @@ pub fn set_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result<(
 }
 
 pub fn get_subdomain(user: &GlobalUser, target: &Target) -> Result<()> {
-    let subdomain = Subdomain::get(&target.account_id, user)?;
+    let subdomain = Subdomain::get(target.account_id.load()?, user)?;
     if let Some(subdomain) = subdomain {
         let msg = format!("{}.workers.dev", subdomain);
         StdOut::info(&msg);

src/commands/whoami.rs

@@ -111,7 +111,7 @@ fn fetch_api_token_email(
 }
 
 /// Fetch the accounts associated with a user
-fn fetch_accounts(user: &GlobalUser) -> Result<Vec<Account>> {
+pub(crate) fn fetch_accounts(user: &GlobalUser) -> Result<Vec<Account>> {
     let client = http::cf_v4_client(user)?;
     let response = client.request(&account::ListAccounts { params: None });
     match response {

src/deploy/schedule.rs

@@ -11,11 +11,15 @@ pub struct ScheduleTarget {
 }
 
 impl ScheduleTarget {
-    pub fn build(account_id: String, script_name: String, crons: Vec<String>) -> Result<Self> {
+    pub fn build(
+        account_id: Option<String>,
+        script_name: String,
+        crons: Vec<String>,
+    ) -> Result<Self> {
         // TODO: add validation for expressions before pushing them to the API
         // we can do this once the cron parser is open sourced
         Ok(Self {
-            account_id,
+            account_id: account_id.unwrap_or_default(),
             script_name,
             crons,
         })