Merge pull request #139 from erickt/lookup

Convert Client::_fetch_target's lookup fn into a method
diff --git a/src/client.rs b/src/client.rs
index 55fc68f..dd8debd 100644
--- a/src/client.rs
+++ b/src/client.rs
@@ -175,8 +175,7 @@
                 &config.max_root_size,
                 config.min_bytes_per_second,
                 None,
-            )
-            .or_else(|_| -> Result<SignedMetadata<_, RootMetadata>> {
+            ).or_else(|_| -> Result<SignedMetadata<_, RootMetadata>> {
                 // FIXME: should we be fetching the latest version instead of version 1?
                 let root = remote.fetch_metadata(
                     &root_path,
@@ -222,9 +221,8 @@
         path: &MetadataPath,
         version: &MetadataVersion,
         metadata: &SignedMetadata<D, M>,
-    )
-    where
-        M: Metadata
+    ) where
+        M: Metadata,
     {
         match self.local.store_metadata(path, version, metadata) {
             Ok(()) => {}
@@ -290,7 +288,11 @@
             return Err(Error::Programming(err_msg.into()));
         }
 
-        self.store_metadata(&root_path, &MetadataVersion::Number(latest_version), &latest_root);
+        self.store_metadata(
+            &root_path,
+            &MetadataVersion::Number(latest_version),
+            &latest_root,
+        );
         self.store_metadata(&root_path, &MetadataVersion::None, &latest_root);
 
         if self.tuf.root().expires() <= &Utc::now() {
@@ -440,151 +442,6 @@
 
     // TODO this should check the local repo first
     fn _fetch_target(&mut self, target: &TargetPath) -> Result<SafeReader<R::TargetRead>> {
-        fn lookup<D_, L_, R_, T_>(
-            tuf: &mut Tuf<D_>,
-            config: &Config<T_>,
-            default_terminate: bool,
-            current_depth: u32,
-            target: &VirtualTargetPath,
-            snapshot: &SnapshotMetadata,
-            targets: Option<&TargetsMetadata>,
-            local: &mut L_,
-            remote: &mut R_,
-        ) -> (bool, Result<TargetDescription>)
-        where
-            D_: DataInterchange,
-            L_: Repository<D_>,
-            R_: Repository<D_>,
-            T_: PathTranslator,
-        {
-            if current_depth > config.max_delegation_depth {
-                warn!(
-                    "Walking the delegation graph would have exceeded the configured max depth: {}",
-                    config.max_delegation_depth
-                );
-                return (default_terminate, Err(Error::NotFound));
-            }
-
-            // these clones are dumb, but we need immutable values and not references for update
-            // tuf in the loop below
-            let targets = match targets {
-                Some(t) => t.clone(),
-                None => match tuf.targets() {
-                    Some(t) => t.clone(),
-                    None => {
-                        return (
-                            default_terminate,
-                            Err(Error::MissingMetadata(Role::Targets)),
-                        )
-                    }
-                },
-            };
-
-            if let Some(t) = targets.targets().get(target) {
-                return (default_terminate, Ok(t.clone()));
-            }
-
-            let delegations = match targets.delegations() {
-                Some(d) => d,
-                None => return (default_terminate, Err(Error::NotFound)),
-            };
-
-            for delegation in delegations.roles().iter() {
-                if !delegation.paths().iter().any(|p| target.is_child(p)) {
-                    if delegation.terminating() {
-                        return (true, Err(Error::NotFound));
-                    } else {
-                        continue;
-                    }
-                }
-
-                let role_meta = match snapshot.meta().get(delegation.role()) {
-                    Some(m) => m,
-                    None if !delegation.terminating() => continue,
-                    None => return (true, Err(Error::NotFound)),
-                };
-
-                let (alg, value) = match crypto::hash_preference(role_meta.hashes()) {
-                    Ok(h) => h,
-                    Err(e) => return (delegation.terminating(), Err(e)),
-                };
-
-                let version = if tuf.root().consistent_snapshot() {
-                    MetadataVersion::Hash(value.clone())
-                } else {
-                    MetadataVersion::None
-                };
-
-                let signed_meta = match local
-                    .fetch_metadata::<TargetsMetadata>(
-                        delegation.role(),
-                        &MetadataVersion::None,
-                        &Some(role_meta.size()),
-                        config.min_bytes_per_second(),
-                        Some((alg, value.clone())),
-                    )
-                    .or_else(|_| {
-                        remote.fetch_metadata::<TargetsMetadata>(
-                            delegation.role(),
-                            &version,
-                            &Some(role_meta.size()),
-                            config.min_bytes_per_second(),
-                            Some((alg, value.clone())),
-                        )
-                    }) {
-                    Ok(m) => m,
-                    Err(ref e) if !delegation.terminating() => {
-                        warn!("Failed to fetch metadata {:?}: {:?}", delegation.role(), e);
-                        continue;
-                    }
-                    Err(e) => {
-                        warn!("Failed to fetch metadata {:?}: {:?}", delegation.role(), e);
-                        return (true, Err(e));
-                    }
-                };
-
-                match tuf.update_delegation(delegation.role(), signed_meta.clone()) {
-                    Ok(_) => {
-                        match local.store_metadata(
-                            delegation.role(),
-                            &MetadataVersion::None,
-                            &signed_meta,
-                        ) {
-                            Ok(_) => (),
-                            Err(e) => warn!(
-                                "Error storing metadata {:?} locally: {:?}",
-                                delegation.role(),
-                                e
-                            ),
-                        }
-
-                        let meta = tuf.delegations().get(delegation.role()).unwrap().clone();
-                        let (term, res) = lookup(
-                            tuf,
-                            config,
-                            delegation.terminating(),
-                            current_depth + 1,
-                            target,
-                            snapshot,
-                            Some(meta.as_ref()),
-                            local,
-                            remote,
-                        );
-
-                        if term && res.is_err() {
-                            return (true, res);
-                        }
-
-                        // TODO end recursion early
-                    }
-                    Err(_) if !delegation.terminating() => continue,
-                    Err(e) => return (true, Err(e)),
-                };
-            }
-
-            (default_terminate, Err(Error::NotFound))
-        }
-
         let virt = self.config.path_translator.real_to_virtual(target)?;
 
         let snapshot = self
@@ -592,17 +449,8 @@
             .snapshot()
             .ok_or_else(|| Error::MissingMetadata(Role::Snapshot))?
             .clone();
-        let (_, target_description) = lookup(
-            &mut self.tuf,
-            &self.config,
-            false,
-            0,
-            &virt,
-            &snapshot,
-            None,
-            &mut self.local,
-            &mut self.remote,
-        );
+        let (_, target_description) =
+            self.lookup_target_description(false, 0, &virt, &snapshot, None);
         let target_description = target_description?;
 
         self.remote.fetch_target(
@@ -611,6 +459,146 @@
             self.config.min_bytes_per_second,
         )
     }
+
+    fn lookup_target_description(
+        &mut self,
+        default_terminate: bool,
+        current_depth: u32,
+        target: &VirtualTargetPath,
+        snapshot: &SnapshotMetadata,
+        targets: Option<&TargetsMetadata>,
+    ) -> (bool, Result<TargetDescription>) {
+        if current_depth > self.config.max_delegation_depth {
+            warn!(
+                "Walking the delegation graph would have exceeded the configured max depth: {}",
+                self.config.max_delegation_depth
+            );
+            return (default_terminate, Err(Error::NotFound));
+        }
+
+        // these clones are dumb, but we need immutable values and not references for update
+        // tuf in the loop below
+        let targets = match targets {
+            Some(t) => t.clone(),
+            None => match self.tuf.targets() {
+                Some(t) => t.clone(),
+                None => {
+                    return (
+                        default_terminate,
+                        Err(Error::MissingMetadata(Role::Targets)),
+                    )
+                }
+            },
+        };
+
+        if let Some(t) = targets.targets().get(target) {
+            return (default_terminate, Ok(t.clone()));
+        }
+
+        let delegations = match targets.delegations() {
+            Some(d) => d,
+            None => return (default_terminate, Err(Error::NotFound)),
+        };
+
+        for delegation in delegations.roles().iter() {
+            if !delegation.paths().iter().any(|p| target.is_child(p)) {
+                if delegation.terminating() {
+                    return (true, Err(Error::NotFound));
+                } else {
+                    continue;
+                }
+            }
+
+            let role_meta = match snapshot.meta().get(delegation.role()) {
+                Some(m) => m,
+                None if !delegation.terminating() => continue,
+                None => return (true, Err(Error::NotFound)),
+            };
+
+            let (alg, value) = match crypto::hash_preference(role_meta.hashes()) {
+                Ok(h) => h,
+                Err(e) => return (delegation.terminating(), Err(e)),
+            };
+
+            let version = if self.tuf.root().consistent_snapshot() {
+                MetadataVersion::Hash(value.clone())
+            } else {
+                MetadataVersion::None
+            };
+
+            let signed_meta = match self
+                .local
+                .fetch_metadata::<TargetsMetadata>(
+                    delegation.role(),
+                    &MetadataVersion::None,
+                    &Some(role_meta.size()),
+                    self.config.min_bytes_per_second(),
+                    Some((alg, value.clone())),
+                ).or_else(|_| {
+                    self.remote.fetch_metadata::<TargetsMetadata>(
+                        delegation.role(),
+                        &version,
+                        &Some(role_meta.size()),
+                        self.config.min_bytes_per_second(),
+                        Some((alg, value.clone())),
+                    )
+                }) {
+                Ok(m) => m,
+                Err(ref e) if !delegation.terminating() => {
+                    warn!("Failed to fetch metadata {:?}: {:?}", delegation.role(), e);
+                    continue;
+                }
+                Err(e) => {
+                    warn!("Failed to fetch metadata {:?}: {:?}", delegation.role(), e);
+                    return (true, Err(e));
+                }
+            };
+
+            match self
+                .tuf
+                .update_delegation(delegation.role(), signed_meta.clone())
+            {
+                Ok(_) => {
+                    match self.local.store_metadata(
+                        delegation.role(),
+                        &MetadataVersion::None,
+                        &signed_meta,
+                    ) {
+                        Ok(_) => (),
+                        Err(e) => warn!(
+                            "Error storing metadata {:?} locally: {:?}",
+                            delegation.role(),
+                            e
+                        ),
+                    }
+
+                    let meta = self
+                        .tuf
+                        .delegations()
+                        .get(delegation.role())
+                        .unwrap()
+                        .clone();
+                    let (term, res) = self.lookup_target_description(
+                        delegation.terminating(),
+                        current_depth + 1,
+                        target,
+                        snapshot,
+                        Some(meta.as_ref()),
+                    );
+
+                    if term && res.is_err() {
+                        return (true, res);
+                    }
+
+                    // TODO end recursion early
+                }
+                Err(_) if !delegation.terminating() => continue,
+                Err(e) => return (true, Err(e)),
+            };
+        }
+
+        (default_terminate, Err(Error::NotFound))
+    }
 }
 
 /// Configuration for a TUF `Client`.
@@ -859,10 +847,11 @@
         snapshot.add_signature(&KEYS[1]).unwrap();
         snapshot.add_signature(&KEYS[2]).unwrap();
 
-        let mut timestamp = TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
-            .unwrap()
-            .signed::<Json>(&KEYS[0])
-            .unwrap();
+        let mut timestamp =
+            TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
+                .unwrap()
+                .signed::<Json>(&KEYS[0])
+                .unwrap();
 
         timestamp.add_signature(&KEYS[1]).unwrap();
         timestamp.add_signature(&KEYS[2]).unwrap();
@@ -941,8 +930,7 @@
                     &None,
                     u32::MAX,
                     None
-                )
-                .unwrap(),
+                ).unwrap(),
         );
 
         ////
@@ -954,8 +942,7 @@
                 &MetadataPath::from_role(&Role::Root),
                 &MetadataVersion::Number(2),
                 &root2,
-            )
-            .unwrap();
+            ).unwrap();
 
         client
             .remote
@@ -963,8 +950,7 @@
                 &MetadataPath::from_role(&Role::Root),
                 &MetadataVersion::None,
                 &root2,
-            )
-            .unwrap();
+            ).unwrap();
 
         client
             .remote
@@ -972,8 +958,7 @@
                 &MetadataPath::from_role(&Role::Root),
                 &MetadataVersion::Number(3),
                 &root3,
-            )
-            .unwrap();
+            ).unwrap();
 
         client
             .remote
@@ -981,8 +966,7 @@
                 &MetadataPath::from_role(&Role::Root),
                 &MetadataVersion::None,
                 &root3,
-            )
-            .unwrap();
+            ).unwrap();
 
         ////
         // Finally, check that the update brings us to version 3.
@@ -1000,8 +984,7 @@
                     &None,
                     u32::MAX,
                     None
-                )
-                .unwrap(),
+                ).unwrap(),
         );
     }
 }
diff --git a/src/crypto.rs b/src/crypto.rs
index 4ba4710..2c9623d 100644
--- a/src/crypto.rs
+++ b/src/crypto.rs
@@ -6,7 +6,7 @@
 use ring::digest::{self, SHA256, SHA512};
 use ring::rand::SystemRandom;
 use ring::signature::{
-    ED25519, Ed25519KeyPair, RSAKeyPair, RSASigningState, RSA_PSS_2048_8192_SHA256,
+    Ed25519KeyPair, RSAKeyPair, RSASigningState, ED25519, RSA_PSS_2048_8192_SHA256,
     RSA_PSS_2048_8192_SHA512, RSA_PSS_SHA256, RSA_PSS_SHA512,
 };
 use serde::de::{Deserialize, Deserializer, Error as DeserializeError};
@@ -521,14 +521,12 @@
                 "rsa_keygen_pubexp:65537",
                 "-outform",
                 "der",
-            ])
-            .output()?;
+            ]).output()?;
 
         let mut pk8 = Command::new("openssl")
             .args(&[
                 "pkcs8", "-inform", "der", "-topk8", "-nocrypt", "-outform", "der",
-            ])
-            .stdin(Stdio::piped())
+            ]).stdin(Stdio::piped())
             .stdout(Stdio::piped())
             .spawn()?;
 
@@ -685,8 +683,9 @@
             .decode(intermediate.public_key().as_bytes())
             .map_err(|e| DeserializeError::custom(format!("{:?}", e)))?;
 
-        let key = PublicKey::from_spki(&bytes, intermediate.scheme().clone())
-            .map_err(|e| DeserializeError::custom(format!("Couldn't parse key as SPKI: {:?}", e)))?;
+        let key = PublicKey::from_spki(&bytes, intermediate.scheme().clone()).map_err(|e| {
+            DeserializeError::custom(format!("Couldn't parse key as SPKI: {:?}", e))
+        })?;
 
         if intermediate.typ() != &key.typ {
             return Err(DeserializeError::custom(format!(
diff --git a/src/lib.rs b/src/lib.rs
index 2db0275..a93a655 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -104,7 +104,13 @@
 #![deny(missing_docs)]
 #![cfg_attr(
     feature = "cargo-clippy",
-    allow(collapsible_if, implicit_hasher, new_ret_no_self, op_ref, too_many_arguments)
+    allow(
+        collapsible_if,
+        implicit_hasher,
+        new_ret_no_self,
+        op_ref,
+        too_many_arguments
+    )
 )]
 
 extern crate chrono;
diff --git a/src/metadata.rs b/src/metadata.rs
index 5f49e43..0e0fb91 100644
--- a/src/metadata.rs
+++ b/src/metadata.rs
@@ -1377,11 +1377,9 @@
                         parents[0]
                             .iter()
                             .any(|p| parent.is_child(p) || parent == &p)
-                    })
-                    .cloned()
+                    }).cloned()
                     .collect::<HashSet<_>>()
-            })
-            .collect::<Vec<_>>();
+            }).collect::<Vec<_>>();
         self.matches_chain(&*new)
     }
 
@@ -1686,12 +1684,11 @@
             return Err(Error::IllegalArgument("Roles cannot be empty.".into()));
         }
 
-        if roles.len()
-            != roles
-                .iter()
-                .map(|r| &r.role)
-                .collect::<HashSet<&MetadataPath>>()
-                .len()
+        if roles.len() != roles
+            .iter()
+            .map(|r| &r.role)
+            .collect::<HashSet<&MetadataPath>>()
+            .len()
         {
             return Err(Error::IllegalArgument(
                 "Cannot have duplicated roles in delegations.".into(),
@@ -1898,8 +1895,7 @@
                         .iter()
                         .map(|p| VirtualTargetPath::new(p.to_string()).unwrap())
                         .collect::<HashSet<_>>()
-                })
-                .collect::<Vec<_>>();
+                }).collect::<Vec<_>>();
             println!(
                 "CASE: expect: {} path: {:?} parents: {:?}",
                 expected, target, parents
@@ -1946,8 +1942,8 @@
             "diNfThTFm0PI8R-Bq7NztUIvZbZiaC_weJBgcqaHlWw=",
             "ar9AgoRsmeEcf6Ponta_1TZu1ds5uXbDemBig30O7ck=",
         ).iter()
-            .map(|k| KeyId::from_string(*k).unwrap())
-            .collect();
+        .map(|k| KeyId::from_string(*k).unwrap())
+        .collect();
         let role_def = RoleDefinition::new(2, hashes).unwrap();
         let jsn = json!({
             "threshold": 2,
@@ -2094,8 +2090,7 @@
                     100,
                     hashmap! { HashAlgorithm::Sha256 => HashValue::new(vec![]) },
                 ).unwrap(),
-            )
-            .build()
+            ).build()
             .unwrap();
 
         let jsn = json!({
@@ -2126,8 +2121,7 @@
             .insert_target_description(
                 VirtualTargetPath::new("foo".into()).unwrap(),
                 TargetDescription::from_reader(&b"foo"[..], &[HashAlgorithm::Sha256]).unwrap(),
-            )
-            .build()
+            ).build()
             .unwrap();
 
         let jsn = json!({
@@ -2215,8 +2209,7 @@
                     100,
                     hashmap! { HashAlgorithm::Sha256 => HashValue::new(vec![]) },
                 ).unwrap(),
-            )
-            .build()
+            ).build()
             .unwrap();
 
         let key = PrivateKey::from_pkcs8(ED25519_1_PK8, SignatureScheme::Ed25519).unwrap();
diff --git a/src/tuf.rs b/src/tuf.rs
index 5ddbc8c..1fa84fa 100644
--- a/src/tuf.rs
+++ b/src/tuf.rs
@@ -769,11 +769,12 @@
             .signed::<Json>(&KEYS[1])
             .unwrap();
 
-        let timestamp = TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
-            .unwrap()
-            // sign it with the root key
-            .signed::<Json>(&KEYS[0])
-            .unwrap();
+        let timestamp =
+            TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
+                .unwrap()
+                // sign it with the root key
+                .signed::<Json>(&KEYS[0])
+                .unwrap();
 
         assert!(tuf.update_timestamp(timestamp).is_err())
     }
@@ -822,11 +823,12 @@
             .signed::<Json>(&KEYS[2])
             .unwrap();
 
-        let timestamp = TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
-            .unwrap()
-            // sign it with the targets key
-            .signed::<Json>(&KEYS[2])
-            .unwrap();
+        let timestamp =
+            TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
+                .unwrap()
+                // sign it with the targets key
+                .signed::<Json>(&KEYS[2])
+                .unwrap();
 
         tuf.update_timestamp(timestamp).unwrap();
 
diff --git a/tests/integration.rs b/tests/integration.rs
index d00c64d..ad495f0 100644
--- a/tests/integration.rs
+++ b/tests/integration.rs
@@ -43,12 +43,10 @@
         .insert_metadata_description(
             MetadataPath::new("targets".into()).unwrap(),
             MetadataDescription::from_reader(&*vec![0u8], 1, &[HashAlgorithm::Sha256]).unwrap(),
-        )
-        .insert_metadata_description(
+        ).insert_metadata_description(
             MetadataPath::new("delegation".into()).unwrap(),
             MetadataDescription::from_reader(&*vec![0u8], 1, &[HashAlgorithm::Sha256]).unwrap(),
-        )
-        .signed::<Json>(&snapshot_key)
+        ).signed::<Json>(&snapshot_key)
         .unwrap();
 
     let timestamp = TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
@@ -92,8 +90,7 @@
             VirtualTargetPath::new("foo".into()).unwrap(),
             target_file,
             &[HashAlgorithm::Sha256],
-        )
-        .unwrap()
+        ).unwrap()
         .signed::<Json>(&delegation_key)
         .unwrap();
 
@@ -133,16 +130,13 @@
         .insert_metadata_description(
             MetadataPath::new("targets".into()).unwrap(),
             MetadataDescription::from_reader(&*vec![0u8], 1, &[HashAlgorithm::Sha256]).unwrap(),
-        )
-        .insert_metadata_description(
+        ).insert_metadata_description(
             MetadataPath::new("delegation-a".into()).unwrap(),
             MetadataDescription::from_reader(&*vec![0u8], 1, &[HashAlgorithm::Sha256]).unwrap(),
-        )
-        .insert_metadata_description(
+        ).insert_metadata_description(
             MetadataPath::new("delegation-b".into()).unwrap(),
             MetadataDescription::from_reader(&*vec![0u8], 1, &[HashAlgorithm::Sha256]).unwrap(),
-        )
-        .signed::<Json>(&snapshot_key)
+        ).signed::<Json>(&snapshot_key)
         .unwrap();
 
     let timestamp = TimestampMetadataBuilder::from_snapshot(&snapshot, &[HashAlgorithm::Sha256])
@@ -220,8 +214,7 @@
             VirtualTargetPath::new("foo".into()).unwrap(),
             target_file,
             &[HashAlgorithm::Sha256],
-        )
-        .unwrap()
+        ).unwrap()
         .signed::<Json>(&delegation_b_key)
         .unwrap();
 
diff --git a/tests/simple_example.rs b/tests/simple_example.rs
index ff7707c..8eaea1c 100644
--- a/tests/simple_example.rs
+++ b/tests/simple_example.rs
@@ -105,8 +105,7 @@
             config.path_translator().real_to_virtual(&target_path)?,
             target_file,
             &[HashAlgorithm::Sha256],
-        )?
-        .signed::<Json>(&targets_key)?;
+        )?.signed::<Json>(&targets_key)?;
 
     remote.store_metadata(
         &MetadataPath::new("targets".into())?,