Removing integration try from and implementing custom validation func…

…tion. Creating methods for BranchImportStrategy and TagImportStrategy

docs/dev/README.md

@@ -200,6 +200,6 @@ In order to allow Broker to scan tags in your remote, `import_tags` must be set
 
 ### toggling fields
 
-Toggling `import_branches` from `true` to `false` will remove all existing uploaded scans for ALL branches of that particular remote in your database. If toggled from `false` to `true`, Broker will perform as if it is scanning the listed `watched_branches` for the first time.
+Toggling `import_branches` from `true` to `false` will remove all existing uploaded scans for ALL branches of that particular remote in your local database (this does NOT delete your scans in the FOSSA UI). If toggled from `false` to `true`, Broker will perform as if it is scanning the listed `watched_branches` for the first time. On subsequent poll cycles, Broker will import the latest changes from your configured branches since the last revision (skipping any intermediate commits).
 
-Toggling `import_tags` from `true` to `false` will remove all existing uploaded scans for ALL tags of that particular remote in your database. If toggled from `false` to `true`, Broker will perform as if it is scanning all the remote's tags for the first time. This would mean that all tags for that remote would be scanned.
+Toggling `import_tags` from `true` to `false` will remove all existing uploaded scans for ALL tags of that particular remote in your local database (this does NOT delete your scans in the FOSSA UI). If toggled from `false` to `true`, Broker will perform as if it is scanning all the remote's tags for the first time. This would mean that all tags for that remote would be scanned. On subsequent poll cycles, Broker will import all created or changed tags since the last poll cycle.

docs/reference/config.md

@@ -74,7 +74,7 @@ This block specifies how to configure Broker to communicate with a git server fo
 | `auth`            | Required  | Required authentication to clone this repository.                                             | N/A               | N/A           |
 | `team`            | Optional  | The team in FOSSA to which this project should be assigned.<sup>2</sup>                       | N/A               | N/A           |
 | `title`           | Optional  | Specify a custom title for the project instead of using the default.<sup>3</sup>              | N/A               | N/A           |
-|`import_branches`  | Optional  | Initialize to scan specific branches for the remote repository                                | N/A               | N/A           |
+| `import_branches` | Optional  | Initialize to scan specific branches for the remote repository                                | N/A               | N/A           |
 | `import_tags`     | Optional  | Initialize to scan tags for the remote repository                                             | N/A               | N/A           |
 | `watched_branches`| Optional  | The name of the branches that you intend to scan                                              | N/A               | N/A           |
 
@@ -155,9 +155,9 @@ In order to allow Broker to scan tags in your remote, `import_tags` must be set
 
 ### toggling fields
 
-Toggling `import_branches` from `true` to `false` will remove all existing uploaded scans for ALL branches of that particular remote in your database. If toggled from `false` to `true`, Broker will perform as if it is scanning the listed `watched_branches` for the first time.
+Toggling `import_branches` from `true` to `false` will remove all existing uploaded scans for ALL branches of that particular remote in your local database (this does NOT delete your scans in the FOSSA UI). If toggled from `false` to `true`, Broker will perform as if it is scanning the listed `watched_branches` for the first time. On subsequent poll cycles, Broker will import the latest changes from your configured branches since the last revision (skipping any intermediate commits).
 
-Toggling `import_tags` from `true` to `false` will remove all existing uploaded scans for ALL tags of that particular remote in your database. If toggled from `false` to `true`, Broker will perform as if it is scanning all the remote's tags for the first time. This would mean that all tags for that remote would be scanned.
+Toggling `import_tags` from `true` to `false` will remove all existing uploaded scans for ALL tags of that particular remote in your local database (this does NOT delete your scans in the FOSSA UI). If toggled from `false` to `true`, Broker will perform as if it is scanning all the remote's tags for the first time. This would mean that all tags for that remote would be scanned. On subsequent poll cycles, Broker will import all created or changed tags since the last poll cycle.
 
 ## Integration authentication
 

src/api/remote.rs

@@ -258,7 +258,7 @@ impl TryFrom<String> for PollInterval {
 }
 
 /// Specificies if we want to scan branches
-#[derive(Debug, Clone, PartialEq, Eq, Display, Deserialize, Serialize, new)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Display, Deserialize, Serialize, new)]
 pub enum BranchImportStrategy {
     /// Scanning branches is not allowed
     Disabled,
@@ -276,8 +276,36 @@ impl From<Option<bool>> for BranchImportStrategy {
     }
 }
 
+impl BranchImportStrategy {
+    /// Validates branch import configurations
+    pub fn is_valid(&self, watched_branches: &[WatchedBranch]) -> bool {
+        match self {
+            BranchImportStrategy::Disabled => watched_branches.is_empty(),
+            // On the Enabled variant, we can't check if watched branches is not empty here because Broker will try to infer watched branches
+            // Therefore just have the Enabled case map to true, and the validation on the enabled case will be checked later
+            BranchImportStrategy::Enabled => true,
+        }
+    }
+
+    /// Checks if we need to infer watched branches based on configuration
+    pub fn infer_watched_branches(&self, watched_branches: &[WatchedBranch]) -> bool {
+        match self {
+            BranchImportStrategy::Disabled => false,
+            BranchImportStrategy::Enabled => watched_branches.is_empty(),
+        }
+    }
+
+    /// Checks if scanning on branches should be skipped based on the enum variant
+    pub fn should_skip_branches(&self) -> bool {
+        match self {
+            BranchImportStrategy::Disabled => true,
+            BranchImportStrategy::Enabled => false,
+        }
+    }
+}
+
 /// Specifies if the we want to scan tags
-#[derive(Debug, Clone, PartialEq, Eq, Display, Deserialize, Serialize, new)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Display, Deserialize, Serialize, new)]
 pub enum TagImportStrategy {
     /// Scanning tags is not allowed
     Disabled,
@@ -295,6 +323,16 @@ impl From<Option<bool>> for TagImportStrategy {
     }
 }
 
+impl TagImportStrategy {
+    /// Checks if scanning tags should be skipped based on the enum variant
+    pub fn should_skip_tags(&self) -> bool {
+        match self {
+            TagImportStrategy::Disabled => true,
+            TagImportStrategy::Enabled => false,
+        }
+    }
+}
+
 /// The integration's branch that you intend to scan
 #[derive(Debug, Clone, PartialEq, Eq, AsRef, Display, Deserialize, Serialize, new)]
 pub struct WatchedBranch(String);

src/cmd/run.rs

@@ -308,13 +308,13 @@ async fn execute_poll_integration<D: Database>(
                     Reference::Git(git_reference) => match git_reference {
                         git::Reference::Branch {..} => {
                             // Skipping because integration is not configured to scan branches or branch was not in the integration's watched branches
-                            if integration.import_branches() == &BranchImportStrategy::Disabled || !integration.should_scan_reference(reference.name()){
+                            if integration.import_branches().should_skip_branches() || !integration.should_scan_reference(reference.name()){
                                 return None
                             }
                         },
                         git::Reference::Tag{..}  => {
                             // Skipping because integration was not configured to scan tags
-                            if let TagImportStrategy::Disabled = integration.import_tags()  {
+                            if integration.import_tags().should_skip_tags() {
                                 return None
                             }
                         },

src/config/file/v1.rs

@@ -4,7 +4,7 @@ use error_stack::{report, Report, ResultExt};
 use futures::future::join_all;
 use serde::Deserialize;
 use std::path::PathBuf;
-use tokio::task;
+use tap::Pipe;
 use tracing::warn;
 
 use crate::{
@@ -76,16 +76,13 @@ async fn validate(config: RawConfigV1) -> Result<super::Config, Report<Error>> {
     let key = fossa::Key::try_from(config.integration_key).change_context(Error::Validate)?;
     let api = fossa::Config::new(endpoint, key);
     let debugging = debug::Config::try_from(config.debugging).change_context(Error::Validate)?;
-
-    // Spawn_blocking is used to allow try_from to operate as a blocking function.
-    // Try_from is a trait and cannot operate as an async function, but spawn_blocking allows you to bypass this.
-    let integrations = config.integrations.into_iter().map(|val| async {
-        task::spawn_blocking(move || remote::Integration::try_from(val)).await
-    });
-    let integrations = join_all(integrations)
+    let integrations = config
+        .integrations
+        .into_iter()
+        .map(|integration| async { remote::Integration::validate(integration).await })
+        .pipe(join_all)
         .await
         .into_iter()
-        .flatten()
         .collect::<Result<Vec<_>, Report<remote::ValidationError>>>()
         .change_context(Error::Validate)
         .map(remote::Integrations::new)?;
@@ -150,14 +147,15 @@ pub(super) enum Integration {
         auth: Auth,
         import_branches: Option<bool>,
         import_tags: Option<bool>,
+        // Option<Vec<T>> is generally not meaningful, because None is generally equivalent to an empty vector.
+        // However, this needs to be an option due to serde deny_unknown_fields.
+        // An empty vector will throw errors, which is not the intended action for users on these new changes
         watched_branches: Option<Vec<String>>,
     },
 }
 
-impl TryFrom<Integration> for remote::Integration {
-    type Error = Report<remote::ValidationError>;
-
-    fn try_from(value: Integration) -> Result<Self, Self::Error> {
+impl remote::Integration {
+    async fn validate(value: Integration) -> Result<Self, Report<remote::ValidationError>> {
         let mut integration = match value {
             Integration::Git {
                 poll_interval,
@@ -179,13 +177,11 @@ impl TryFrom<Integration> for remote::Integration {
                     .map(remote::WatchedBranch::new)
                     .collect::<Vec<_>>();
 
-                if import_branches == remote::BranchImportStrategy::Disabled
-                    && !watched_branches.is_empty()
-                {
-                    report!(remote::ValidationError::ImportBranches)
+                if !import_branches.is_valid(&watched_branches) {
+                    return report!(remote::ValidationError::ImportBranches)
                         .wrap_err()
                         .help("import branches must be 'true' if watched branches are provided")
-                        .describe_lazy(|| "import branches: 'false'".to_string())?
+                        .describe_lazy(|| "import branches: 'false'".to_string());
                 }
 
                 let protocol = match auth {
@@ -233,11 +229,11 @@ impl TryFrom<Integration> for remote::Integration {
             }
         };
 
-        if integration.watched_branches().is_empty()
-            && integration.import_branches() == &remote::BranchImportStrategy::Enabled
+        if integration
+            .import_branches()
+            .infer_watched_branches(integration.watched_branches())
         {
-            let references =
-                futures::executor::block_on(integration.references()).unwrap_or_default();
+            let references = integration.references().await.unwrap_or_default();
             let primary_branch = references
                 .iter()
                 .find(|r| r.name() == MAIN_BRANCH || r.name() == MASTER_BRANCH)
@@ -254,7 +250,7 @@ impl TryFrom<Integration> for remote::Integration {
                 }
                 Some(branch) => {
                     let primary_branch_name = branch.name();
-                    warn!("Inferred '{primary_branch_name}' as the primary branch for '{integration:#?}'. Broker imports only the primary branch when inferred; if desired this can be customized with the integration configuration.");
+                    warn!("Inferred '{primary_branch_name}' as the primary branch for '{integration}'. Broker imports only the primary branch when inferred; if desired this can be customized with the integration configuration.");
                     let watched_branch = remote::WatchedBranch::new(branch.name().to_string());
                     integration.add_watched_branch(watched_branch);
                 }

src/db/sqlite.rs

@@ -38,7 +38,6 @@ use sqlx::{
 };
 use tap::TapFallible;
 use thiserror::Error;
-use tracing::warn;
 
 use crate::{
     doc::{crate_name, crate_version},
@@ -218,12 +217,10 @@ impl super::Database for Database {
                         "})
                     .help("try again with the latest version of Broker")
             }
-            Some(db_version) if current_version > db_version => {
-                warn!("Broker currently runnning on {db_version:?}. Updating Broker to run on {current_version:?}");
-                self.update_db_version(&current_version)
-                    .await
-                    .change_context(super::Error::Interact)
-            }
+            Some(db_version) if current_version > db_version => self
+                .update_db_version(&current_version)
+                .await
+                .change_context(super::Error::Interact),
             Some(_) => Ok(()),
         }
     }