ref: Separate LegacyUploadContext for legacy uploads

Legacy (i.e. non-chunked) are distinct from chunked uploads, so it makes sense to have a separate struct to enforce these differences via the type system.

Specifically, legacy uploads must be associated with a release, whereas this is not required for many chunk uploads. Legacy uploads must also be associated with at most one project, whereas chunked uploads can have multiple projects (support for which we will add in the CLI with #2408)

Author: szokeasaurusrex

src/api/mod.rs

@@ -47,7 +47,7 @@ use uuid::Uuid;
 use crate::api::errors::ProjectRenamedError;
 use crate::config::{Auth, Config};
 use crate::constants::{ARCH, DEFAULT_URL, EXT, PLATFORM, RELEASE_REGISTRY_LATEST_URL, VERSION};
-use crate::utils::file_upload::UploadContext;
+use crate::utils::file_upload::LegacyUploadContext;
 use crate::utils::http::{self, is_absolute_url};
 use crate::utils::progress::{ProgressBar, ProgressBarMode};
 use crate::utils::retry::{get_default_backoff, DurationAsMilliseconds};
@@ -1425,28 +1425,24 @@ impl RegionSpecificApi<'_> {
     /// system and uploaded as `name`.
     pub fn upload_release_file(
         &self,
-        context: &UploadContext,
+        context: &LegacyUploadContext,
         contents: &[u8],
         name: &str,
         headers: Option<&[(String, String)]>,
         progress_bar_mode: ProgressBarMode,
     ) -> ApiResult<Option<Artifact>> {
-        let release = context
-            .release()
-            .map_err(|err| ApiError::with_source(ApiErrorKind::ReleaseNotFound, err))?;
-
-        let path = if let Some(project) = context.project {
+        let path = if let Some(project) = context.project() {
             format!(
                 "/projects/{}/{}/releases/{}/files/",
-                PathArg(context.org),
+                PathArg(context.org()),
                 PathArg(project),
-                PathArg(release)
+                PathArg(context.release())
             )
         } else {
             format!(
                 "/organizations/{}/releases/{}/files/",
-                PathArg(context.org),
-                PathArg(release)
+                PathArg(context.org()),
+                PathArg(context.release())
             )
         };
         let mut form = curl::easy::Form::new();
@@ -1459,7 +1455,7 @@ impl RegionSpecificApi<'_> {
             .buffer(filename, contents.to_vec())
             .add()?;
         form.part("name").contents(name.as_bytes()).add()?;
-        if let Some(dist) = context.dist {
+        if let Some(dist) = context.dist() {
             form.part("dist").contents(dist.as_bytes()).add()?;
         }
 

src/commands/files/upload.rs

@@ -244,7 +244,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
         if let Some(artifact) = authenticated_api
             .region_specific(context.org)
             .upload_release_file(
-                context,
+                &context.try_into()?,
                 &contents,
                 name,
                 Some(

src/utils/file_upload.rs

@@ -1,6 +1,6 @@
 //! Searches, processes and uploads release files.
 use std::collections::{BTreeMap, HashMap};
-use std::fmt;
+use std::fmt::{self, Display};
 use std::io::BufWriter;
 use std::path::PathBuf;
 use std::str;
@@ -19,6 +19,7 @@ use symbolic::common::ByteView;
 use symbolic::debuginfo::sourcebundle::{
     SourceBundleErrorKind, SourceBundleWriter, SourceFileInfo, SourceFileType,
 };
+use thiserror::Error;
 use url::Url;
 
 use crate::api::NewRelease;
@@ -100,6 +101,89 @@ impl UploadContext<'_> {
     }
 }
 
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum LegacyUploadContextError {
+    #[error("A release is required for this upload")]
+    ReleaseMissing,
+}
+
+#[derive(Debug, Default)]
+pub struct LegacyUploadContext<'a> {
+    org: &'a str,
+    project: Option<&'a str>,
+    release: &'a str,
+    dist: Option<&'a str>,
+}
+
+impl LegacyUploadContext<'_> {
+    pub fn org(&self) -> &str {
+        self.org
+    }
+
+    pub fn project(&self) -> Option<&str> {
+        self.project
+    }
+
+    pub fn release(&self) -> &str {
+        self.release
+    }
+
+    pub fn dist(&self) -> Option<&str> {
+        self.dist
+    }
+}
+
+impl Display for LegacyUploadContext<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        writeln!(
+            f,
+            "{} {}",
+            style("> Organization:").dim(),
+            style(self.org).yellow()
+        )?;
+        writeln!(
+            f,
+            "{} {}",
+            style("> Project:").dim(),
+            style(self.project.unwrap_or("None")).yellow()
+        )?;
+        writeln!(
+            f,
+            "{} {}",
+            style("> Release:").dim(),
+            style(self.release).yellow()
+        )?;
+        writeln!(
+            f,
+            "{} {}",
+            style("> Dist:").dim(),
+            style(self.dist.unwrap_or("None")).yellow()
+        )?;
+        write!(
+            f,
+            "{} {}",
+            style("> Upload type:").dim(),
+            style("single file/legacy upload").yellow()
+        )
+    }
+}
+
+impl<'a> TryFrom<&'a UploadContext<'_>> for LegacyUploadContext<'a> {
+    type Error = LegacyUploadContextError;
+
+    fn try_from(value: &'a UploadContext) -> Result<Self, Self::Error> {
+        Ok(LegacyUploadContext {
+            org: value.org,
+            project: value.project,
+            release: value
+                .release
+                .ok_or(LegacyUploadContextError::ReleaseMissing)?,
+            dist: value.dist,
+        })
+    }
+}
+
 #[derive(Eq, PartialEq, Debug, Copy, Clone)]
 pub enum LogLevel {
     Warning,
@@ -215,7 +299,7 @@ impl<'a> FileUpload<'a> {
             .context
             .chunk_upload_options
             .map_or(DEFAULT_CONCURRENCY, |o| usize::from(o.concurrency));
-        upload_files_parallel(self.context, &self.files, concurrency)
+        upload_files_parallel(&self.context.try_into()?, &self.files, concurrency)
     }
 
     pub fn build_jvm_bundle(&self, debug_id: Option<DebugId>) -> Result<TempFile> {
@@ -224,18 +308,18 @@ impl<'a> FileUpload<'a> {
 }
 
 fn upload_files_parallel(
-    context: &UploadContext,
+    context: &LegacyUploadContext,
     files: &SourceFiles,
     num_threads: usize,
 ) -> Result<()> {
     let api = Api::current();
-    let release = context.release()?;
+    let release = context.release();
 
     // get a list of release files first so we know the file IDs of
     // files that already exist.
     let release_files: HashMap<_, _> = api
         .authenticated()?
-        .list_release_files(context.org, context.project, release)?
+        .list_release_files(context.org, context.project(), release)?
         .into_iter()
         .map(|artifact| ((artifact.dist, artifact.name), artifact.id))
         .collect();
@@ -308,7 +392,7 @@ fn upload_files_parallel(
 
     pb.finish_and_clear();
 
-    print_upload_context_details(context);
+    println!("{}", context);
 
     Ok(())
 }