refactor resource discovery code to use new helper functions

Author: SteveL-MSFT

dsc_lib/src/discovery/command_discovery.rs

@@ -17,7 +17,7 @@ use std::ffi::OsStr;
 use std::fs::File;
 use std::io::BufReader;
 use std::path::{Path, PathBuf};
-use tracing::{debug, error, info, trace, warn, warn_span, Span};
+use tracing::{debug, info, trace, warn, warn_span};
 use tracing_indicatif::span_ext::IndicatifSpanExt;
 
 pub struct CommandDiscovery {
@@ -273,144 +273,52 @@ impl ResourceDiscovery for CommandDiscovery {
         Ok(resources)
     }
 
-    #[allow(clippy::too_many_lines)]
+    // TODO: handle version requirements
     fn find_resources(&mut self, required_resource_types: &[String]) -> Result<BTreeMap<String, DscResource>, DscError>
     {
         debug!("Searching for resources: {:?}", required_resource_types);
+        self.discover_resources("*")?;
 
-        let pb_span = warn_span!("");
-        pb_span.pb_set_style(&ProgressStyle::with_template(
-            "{spinner:.green} [{elapsed_precise:.cyan}] [{bar:40.cyan/blue}] {pos:>7}/{len:7} {msg:.yellow}"
-        )?);
-        pb_span.pb_set_message("Searching for resources");
-        let _ = pb_span.enter();
-
-        let mut resources: BTreeMap<String, DscResource> = BTreeMap::new();
-        let mut adapter_resources: BTreeMap<String, DscResource> = BTreeMap::new();
+        let mut found_resources = BTreeMap::<String, DscResource>::new();
         let mut remaining_required_resource_types = required_resource_types.to_owned();
 
-        if let Ok(paths) = CommandDiscovery::get_resource_paths() {
-            for path in paths {
-                debug!("Searching in {:?}", path);
-                if path.exists() && path.is_dir() {
-                    for entry in path.read_dir().unwrap() {
-                        let entry = entry.unwrap();
-                        let path = entry.path();
-                        if path.is_file() {
-                            let file_name = path.file_name().unwrap().to_str().unwrap();
-                            let file_name_lowercase = file_name.to_lowercase();
-                            if file_name_lowercase.ends_with(".dsc.resource.json") ||
-                            file_name_lowercase.ends_with(".dsc.resource.yaml") ||
-                            file_name_lowercase.ends_with(".dsc.resource.yml") {
-                                let resource = match load_manifest(&path)
-                                {
-                                    Ok(r) => r,
-                                    Err(e) => {
-                                        /*  In case of non-list resource/config operations:
-                                            At this point we can't determine whether or not the bad manifest contains resource that is requested by resource/config operation
-                                            if it is, then "ResouceNotFound" error will be issued later
-                                            and here we just record the error into debug stream.*/
-                                        debug!("{}", e);
-                                        continue;
-                                    },
-                                };
+        for (resource_name, resources) in self.resources.iter() {
+            let Some(resource ) = resources.first() else {
+                // skip if no resources
+                continue;
+            };
 
-                                if let Some(ref manifest) = resource.manifest {
-                                    let manifest = import_manifest(manifest.clone())?;
-                                    if manifest.kind == Some(Kind::Adapter) {
-                                        adapter_resources.insert(resource.type_name.to_lowercase(), resource.clone());
-                                        resources.insert(resource.type_name.to_lowercase(), resource.clone());
-                                    }
-                                }
-                                if remaining_required_resource_types.contains(&resource.type_name.to_lowercase())
-                                {
-                                    remaining_required_resource_types.retain(|x| *x != resource.type_name.to_lowercase());
-                                    debug!("Found {} in {}", &resource.type_name, path.display());
-                                    Span::current().pb_inc(1);
-                                    resources.insert(resource.type_name.to_lowercase(), resource);
-                                    if remaining_required_resource_types.is_empty()
-                                    {
-                                        return Ok(resources);
-                                    }
-                                }
-                            }
-                        }
-                    }
+            if remaining_required_resource_types.contains(&resource_name.to_lowercase())
+            {
+                // remove the resource from the list of required resources
+                remaining_required_resource_types.retain(|x| *x != resource_name.to_lowercase());
+                found_resources.insert(resource_name.to_lowercase(), resource.clone());
+                if remaining_required_resource_types.is_empty()
+                {
+                    return Ok(found_resources);
                 }
             }
         }
-        debug!("Found {} matching non-adapter-based resources", resources.len());
+        debug!("Found {} matching non-adapter-based resources", found_resources.len());
 
         // now go through the adapter resources and add them to the list of resources
-        for adapter in adapter_resources {
-            debug!("Enumerating resources for adapter '{}'", adapter.1.type_name);
-            let pb_adapter_span = warn_span!("");
-            pb_adapter_span.pb_set_style(&ProgressStyle::with_template(
-                "{spinner:.green} [{elapsed_precise:.cyan}] {msg:.white}"
-            )?);
-            pb_adapter_span.pb_set_message(format!("Enumerating resources for adapter '{}'", adapter.1.type_name).as_str());
-            let _ = pb_adapter_span.enter();
-            let adapter_resource = adapter.1;
-            let adapter_type_name = adapter_resource.type_name.clone();
-            let manifest = if let Some(manifest) = adapter_resource.manifest {
-                if let Ok(manifest) = import_manifest(manifest) {
-                    manifest
-                } else {
-                    return Err(DscError::Operation(format!("Failed to import manifest for '{}'", adapter_resource.type_name.clone())));
-                }
-            } else {
-                return Err(DscError::MissingManifest(adapter_resource.type_name.clone()));
+        for (adapted_name, adapted_resource) in self.adapted_resources.iter() {
+            let Some(adapted_resource) = adapted_resource.first() else {
+                // skip if no resources
+                continue;
             };
-            let mut adapter_resources_count = 0;
-            // invoke the list command
-            let list_command = manifest.adapter.unwrap().list;
-            let (exit_code, stdout, stderr) = match invoke_command(&list_command.executable, list_command.args, None, Some(&adapter_resource.directory), None)
-            {
-                Ok((exit_code, stdout, stderr)) => (exit_code, stdout, stderr),
-                Err(e) => {
-                    /* In case of non-list resource/config operations:
-                       print failure from adapter as error because this adapter was specifically requested by current resource/config operation*/
-                    error!("Could not start {}: {}", list_command.executable, e);
-                    continue;
-                },
-            };
-            log_resource_traces(&stderr);
 
-            if exit_code != 0 {
-                 /* In case of non-list resource/config operations:
-                    print failure from adapter as error because this adapter was specifically requested by current resource/config operation*/
-                error!("Adapter failed to list resources with exit code {exit_code}: {stderr}");
-            }
-
-            for line in stdout.lines() {
-                match serde_json::from_str::<DscResource>(line){
-                    Result::Ok(resource) => {
-                        if resource.require_adapter.is_none() {
-                            error!("{}", DscError::MissingRequires(adapter.0.clone(), resource.type_name.clone()).to_string());
-                            continue;
-                        }
-                        if remaining_required_resource_types.contains(&resource.type_name.to_lowercase())
-                        {
-                            remaining_required_resource_types.retain(|x| *x != resource.type_name.to_lowercase());
-                            debug!("Found {} in {}", &resource.type_name, &resource.path);
-                            resources.insert(resource.type_name.to_lowercase(), resource);
-                            adapter_resources_count += 1;
-                            if remaining_required_resource_types.is_empty()
-                            {
-                                return Ok(resources);
-                            }
-                        }
-                    },
-                    Result::Err(err) => {
-                        error!("Failed to parse resource: {line} -> {err}");
-                        continue;
-                    }
-                };
+            if remaining_required_resource_types.contains(&adapted_name.to_lowercase())
+            {
+                remaining_required_resource_types.retain(|x| *x != adapted_name.to_lowercase());
+                found_resources.insert(adapted_name.to_lowercase(), adapted_resource.clone());
+                if remaining_required_resource_types.is_empty()
+                {
+                    return Ok(found_resources);
+                }
             }
-
-            debug!("Adapter '{}' listed {} matching resources", adapter_type_name, adapter_resources_count);
         }
-        Ok(resources)
+        Ok(found_resources)
     }
 }
 
@@ -423,8 +331,15 @@ fn insert_resource(resources: &mut BTreeMap<String, Vec<DscResource>>, resource:
         };
         // compare the resource versions and insert newest to oldest using semver
         let mut insert_index = resource_versions.len();
-        for (index, resource_version) in resource_versions.iter().enumerate() {
-            if Version::parse(&resource_version.version)? < Version::parse(&resource.version)? {
+        for (index, resource_instance) in resource_versions.iter().enumerate() {
+            let resource_instance_version = Version::parse(&resource_instance.version)?;
+            let resource_version = Version::parse(&resource.version)?;
+            // if the version already exists, we skip
+            if resource_instance_version == resource_version {
+                return Ok(());
+            }
+
+            if resource_instance_version < resource_version {
                 insert_index = index;
                 break;
             }

dsc_lib/src/discovery/mod.rs

@@ -27,7 +27,6 @@ impl Discovery {
     }
 
     /// List operation.
-    #[allow(clippy::missing_panics_doc)] // false positive in clippy; this function will never panic
     pub fn list_available_resources(&mut self, type_name_filter: &str, adapter_name_filter: &str) -> Vec<DscResource> {
         let discovery_types: Vec<Box<dyn ResourceDiscovery>> = vec![
             Box::new(command_discovery::CommandDiscovery::new()),
@@ -45,12 +44,10 @@ impl Discovery {
                 }
             };
 
-            for (_resource_name, resource) in discovered_resources {
-                let Some(resource) = resource.first() else {
-                    continue;
-                };
-
-                resources.push(resource.clone());
+            for (_resource_name, found_resources) in discovered_resources {
+                for resource in found_resources {
+                    resources.push(resource.clone());
+                }
             };
         }