Move Handler's ArtifactDirectory implementation to a separate type

Passing around `self` risks confusing the borrow checker if we need to
invoke a method on a field, and it's rather confusing to begin with.

Author: refi64

src/handler.rs

@@ -228,6 +228,51 @@ pub struct HandlerOptions {
     pub log_tail: u64,
 }
 
+struct GitLabArtifacts<'a> {
+    job: &'a Job,
+    artifacts: &'a mut HashMap<Utf8PathBuf, ArtifactReader>,
+}
+
+#[async_trait]
+impl ArtifactDirectory for GitLabArtifacts<'_> {
+    #[instrument(skip(self, path), path = path.as_ref())]
+    async fn open(&self, path: impl AsRef<Utf8Path> + Send) -> Result<ArtifactReader> {
+        let path = path.as_ref();
+
+        if let Some(file) = self.artifacts.get(path) {
+            let file = file
+                .try_clone()
+                .await
+                .wrap_err("Failed to reopen artifact")?;
+            return Ok(file);
+        }
+
+        for dep in self.job.dependencies() {
+            if let Some(file) = check_for_artifact(dep, path).await? {
+                return Ok(file);
+            }
+        }
+
+        Err(MissingArtifact(path.to_owned()).into())
+    }
+
+    #[tracing::instrument(skip(self, path, func), path = path.as_ref())]
+    async fn save_with<Ret, Err, F, P>(&mut self, path: P, func: F) -> Result<Ret>
+    where
+        Report: From<Err>,
+        Ret: Send,
+        Err: Send,
+        F: for<'a> SaveCallback<'a, Ret, Err> + Send,
+        P: AsRef<Utf8Path> + Send,
+    {
+        let mut writer = ArtifactWriter::new().await?;
+        let ret = func(&mut writer).await?;
+        self.artifacts
+            .insert(path.as_ref().to_owned(), writer.into_reader().await?);
+        Ok(ret)
+    }
+}
+
 pub struct ObsJobHandler {
     job: Job,
     client: obs::Client,
@@ -289,6 +334,11 @@ impl ObsJobHandler {
 
     #[instrument(skip(self))]
     async fn run_dput(&mut self, args: DputAction) -> Result<()> {
+        let mut artifacts = GitLabArtifacts {
+            job: &self.job,
+            artifacts: &mut self.artifacts,
+        };
+
         let branch_to = if !args.branch_to.is_empty() {
             Some(args.branch_to)
         } else {
@@ -305,7 +355,7 @@ impl ObsJobHandler {
             args.project.clone(),
             branch_to,
             args.dsc.as_str().into(),
-            self,
+            &artifacts,
         )
         .await?;
 
@@ -318,7 +368,10 @@ impl ObsJobHandler {
             enabled_repos: HashMap::new(),
         };
         debug!("Saving initial build info: {:?}", build_info);
-        build_info.clone().save(self, &args.build_info_out).await?;
+        build_info
+            .clone()
+            .save(&mut artifacts, &args.build_info_out)
+            .await?;
 
         let initial_build_meta = BuildMeta::get_if_package_exists(
             self.client.clone(),
@@ -334,7 +387,7 @@ impl ObsJobHandler {
         .await?;
         debug!(?initial_build_meta);
 
-        let result = uploader.upload_package(self).await?;
+        let result = uploader.upload_package(&artifacts).await?;
 
         // If we couldn't get the metadata before because the package didn't
         // exist yet, get it now but without history, so we leave the previous
@@ -392,14 +445,21 @@ impl ObsJobHandler {
             ..build_info
         };
         debug!("Saving complete build info: {:?}", build_info);
-        build_info.save(self, &args.build_info_out).await?;
+        build_info
+            .save(&mut artifacts, &args.build_info_out)
+            .await?;
 
         Ok(())
     }
 
     #[instrument(skip(self))]
     async fn run_generate_monitor(&mut self, args: GenerateMonitorAction) -> Result<()> {
-        let build_info_data = self.read_string(&args.build_info).await?;
+        let mut artifacts = GitLabArtifacts {
+            job: &self.job,
+            artifacts: &mut self.artifacts,
+        };
+
+        let build_info_data = artifacts.read_string(&args.build_info).await?;
         let build_info: ObsBuildInfo = serde_yaml::from_str(&build_info_data)
             .wrap_err("Failed to parse provided build info file")?;
 
@@ -432,14 +492,21 @@ impl ObsJobHandler {
             },
         )?;
 
-        self.write(&args.pipeline_out, pipeline.as_bytes()).await?;
+        artifacts
+            .write(&args.pipeline_out, pipeline.as_bytes())
+            .await?;
         outputln!("Wrote pipeline file '{}'.", args.pipeline_out);
 
         Ok(())
     }
 
     #[instrument(skip(self))]
     async fn run_monitor(&mut self, args: MonitorAction) -> Result<()> {
+        let mut artifacts = GitLabArtifacts {
+            job: &self.job,
+            artifacts: &mut self.artifacts,
+        };
+
         let monitor = ObsMonitor::new(
             self.client.clone(),
             MonitoredPackage {
@@ -459,7 +526,7 @@ impl ObsJobHandler {
         debug!("Completed with: {:?}", completion);
 
         let mut log_file = monitor
-            .download_build_log(&args.build_log_out, self)
+            .download_build_log(&args.build_log_out, &mut artifacts)
             .await?;
 
         match completion {
@@ -506,13 +573,18 @@ impl ObsJobHandler {
 
     #[instrument(skip(self))]
     async fn run_download_binaries(&mut self, args: DownloadBinariesAction) -> Result<()> {
+        let mut artifacts = GitLabArtifacts {
+            job: &self.job,
+            artifacts: &mut self.artifacts,
+        };
+
         let binaries = download_binaries(
             self.client.clone(),
             &args.project,
             &args.package,
             &args.repository,
             &args.arch,
-            self,
+            &mut artifacts,
             &args.build_results_dir,
         )
         .await?;
@@ -528,8 +600,13 @@ impl ObsJobHandler {
             return Ok(());
         }
 
+        let artifacts = GitLabArtifacts {
+            job: &self.job,
+            artifacts: &mut self.artifacts,
+        };
+
         let build_info_data = if args.ignore_missing_build_info {
-            if let Some(build_info_data) = self
+            if let Some(build_info_data) = artifacts
                 .read_string(&args.build_info)
                 .await
                 .missing_artifact_to_none()?
@@ -543,7 +620,7 @@ impl ObsJobHandler {
                 return Ok(());
             }
         } else {
-            self.read_string(&args.build_info).await?
+            artifacts.read_string(&args.build_info).await?
         };
 
         let build_info: ObsBuildInfo = serde_yaml::from_str(&build_info_data)
@@ -701,46 +778,6 @@ async fn check_for_artifact(
     Ok(None)
 }
 
-#[async_trait]
-impl ArtifactDirectory for ObsJobHandler {
-    #[instrument(skip(self, path), path = path.as_ref())]
-    async fn open(&self, path: impl AsRef<Utf8Path> + Send) -> Result<ArtifactReader> {
-        let path = path.as_ref();
-
-        if let Some(file) = self.artifacts.get(path) {
-            let file = file
-                .try_clone()
-                .await
-                .wrap_err("Failed to reopen artifact")?;
-            return Ok(file);
-        }
-
-        for dep in self.job.dependencies() {
-            if let Some(file) = check_for_artifact(dep, path).await? {
-                return Ok(file);
-            }
-        }
-
-        Err(MissingArtifact(path.to_owned()).into())
-    }
-
-    #[tracing::instrument(skip(self, path, func), path = path.as_ref())]
-    async fn save_with<Ret, Err, F, P>(&mut self, path: P, func: F) -> Result<Ret>
-    where
-        Report: From<Err>,
-        Ret: Send,
-        Err: Send,
-        F: for<'a> SaveCallback<'a, Ret, Err> + Send,
-        P: AsRef<Utf8Path> + Send,
-    {
-        let mut writer = ArtifactWriter::new().await?;
-        let ret = func(&mut writer).await?;
-        self.artifacts
-            .insert(path.as_ref().to_owned(), writer.into_reader().await?);
-        Ok(ret)
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use std::{