feat: Don't upload chunks that are already on the server

src/utils/file_upload.rs

@@ -274,39 +274,12 @@ fn upload_files_parallel(
     Ok(())
 }
 
-fn upload_files_chunked(
+fn poll_assemble(
+    checksum: Digest,
+    chunks: &[Digest],
     context: &UploadContext,
-    files: &SourceFiles,
     options: &ChunkUploadOptions,
 ) -> Result<()> {
-    let archive = build_artifact_bundle(context, files, None)?;
-
-    let progress_style =
-        ProgressStyle::default_spinner().template("{spinner} Optimizing bundle for upload...");
-
-    let pb = ProgressBar::new_spinner();
-    pb.enable_steady_tick(100);
-    pb.set_style(progress_style);
-
-    let view = ByteView::open(archive.path())?;
-    let (checksum, checksums) = get_sha1_checksums(&view, options.chunk_size)?;
-    let chunks = view
-        .chunks(options.chunk_size as usize)
-        .zip(checksums.iter())
-        .map(|(data, checksum)| Chunk((*checksum, data)))
-        .collect::<Vec<_>>();
-
-    pb.finish_with_duration("Optimizing");
-
-    let progress_style = ProgressStyle::default_bar().template(&format!(
-        "{} Uploading files...\
-       \n{{wide_bar}}  {{bytes}}/{{total_bytes}} ({{eta}})",
-        style(">").dim(),
-    ));
-
-    upload_chunks(&chunks, options, progress_style)?;
-    println!("{} Uploaded files to Sentry", style(">").dim());
-
     let progress_style = ProgressStyle::default_spinner().template("{spinner} Processing files...");
 
     let pb = ProgressBar::new_spinner();
@@ -320,21 +293,21 @@ fn upload_files_chunked(
     };
 
     let api = Api::current();
+    let use_artifact_bundle =
+        options.supports(ChunkUploadCapability::ArtifactBundles) && context.project.is_some();
     let response = loop {
         // prefer standalone artifact bundle upload over legacy release based upload
-        let response = if options.supports(ChunkUploadCapability::ArtifactBundles)
-            && context.project.is_some()
-        {
+        let response = if use_artifact_bundle {
             api.assemble_artifact_bundle(
                 context.org,
                 vec![context.project.unwrap().to_string()],
                 checksum,
-                &checksums,
+                chunks,
                 context.release,
                 context.dist,
             )?
         } else {
-            api.assemble_release_artifacts(context.org, context.release()?, checksum, &checksums)?
+            api.assemble_release_artifacts(context.org, context.release()?, checksum, chunks)?
         };
 
         // Poll until there is a response, unless the user has specified to skip polling. In
@@ -376,6 +349,66 @@ fn upload_files_chunked(
     Ok(())
 }
 
+fn upload_files_chunked(
+    context: &UploadContext,
+    files: &SourceFiles,
+    options: &ChunkUploadOptions,
+) -> Result<()> {
+    let archive = build_artifact_bundle(context, files, None)?;
+
+    let progress_style =
+        ProgressStyle::default_spinner().template("{spinner} Optimizing bundle for upload...");
+
+    let pb = ProgressBar::new_spinner();
+    pb.enable_steady_tick(100);
+    pb.set_style(progress_style);
+
+    let view = ByteView::open(archive.path())?;
+    let (checksum, checksums) = get_sha1_checksums(&view, options.chunk_size)?;
+    let mut chunks = view
+        .chunks(options.chunk_size as usize)
+        .zip(checksums.iter())
+        .map(|(data, checksum)| Chunk((*checksum, data)))
+        .collect::<Vec<_>>();
+
+    pb.finish_with_duration("Optimizing");
+
+    let progress_style = ProgressStyle::default_bar().template(&format!(
+        "{} Uploading files...\
+       \n{{wide_bar}}  {{bytes}}/{{total_bytes}} ({{eta}})",
+        style(">").dim(),
+    ));
+
+    // Filter out chunks that are already on the server. This only matters if we're in the
+    // `assemble_artifact_bundle` case; `assemble_release_artifacts` always returns `missing_chunks: []`,
+    // so there's no point querying the endpoint.
+    if options.supports(ChunkUploadCapability::ArtifactBundles) && context.project.is_some() {
+        let api = Api::current();
+        let response = api.assemble_artifact_bundle(
+            context.org,
+            vec![context.project.unwrap().to_string()],
+            checksum,
+            &checksums,
+            context.release,
+            context.dist,
+        )?;
+        chunks.retain(|Chunk((digest, _))| response.missing_chunks.contains(digest));
+    };
+
+    upload_chunks(&chunks, options, progress_style)?;
+
+    if !chunks.is_empty() {
+        println!("{} Uploaded files to Sentry", style(">").dim());
+        poll_assemble(checksum, &checksums, context, options)
+    } else {
+        println!(
+            "{} Nothing to upload, all files are on the server",
+            style(">").dim()
+        );
+        Ok(())
+    }
+}
+
 fn build_debug_id(files: &SourceFiles) -> DebugId {
     let mut sorted_files = Vec::from_iter(files);
     sorted_files.sort_by_key(|x| x.0);

tests/integration/debug_files/bundle_jvm.rs

@@ -21,7 +21,8 @@ fn command_bundle_jvm_out_not_found_creates_dir() {
         testcase_cwd_path.join("jvm"),
     )
     .unwrap();
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     register_test("debug_files/debug_files-bundle-jvm-output-not-found.trycmd");
 }
 
@@ -35,20 +36,23 @@ fn command_bundle_jvm_fails_out_is_file() {
     }
     copy_recursively("tests/integration/_fixtures/jvm/", testcase_cwd_path).unwrap();
     write(testcase_cwd_path.join("file.txt"), "some file content").unwrap();
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
 
     register_test("debug_files/debug_files-bundle-jvm-output-is-file.trycmd");
 }
 
 #[test]
 fn command_bundle_jvm_fails_input_not_found() {
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     register_test("debug_files/debug_files-bundle-jvm-input-not-found.trycmd");
 }
 
 #[test]
 fn command_bundle_jvm_fails_input_is_file() {
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     register_test("debug_files/debug_files-bundle-jvm-input-is-file.trycmd");
 }
 
@@ -62,13 +66,15 @@ fn command_bundle_jvm_input_dir_empty() {
     }
     copy_recursively("tests/integration/_fixtures/jvm/", testcase_cwd_path).unwrap();
     create_dir(testcase_cwd_path.join("empty-dir")).unwrap();
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     register_test("debug_files/debug_files-bundle-jvm-input-dir-empty.trycmd");
 }
 
 #[test]
 fn command_bundle_jvm_fails_invalid_uuid() {
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     register_test("debug_files/debug_files-bundle-jvm-invalid-uuid.trycmd");
 }
 
@@ -79,6 +85,7 @@ fn command_bundle_jvm() {
         remove_dir_all(testcase_cwd_path).unwrap();
     }
     copy_recursively("tests/integration/_fixtures/jvm/", testcase_cwd_path).unwrap();
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     register_test("debug_files/debug_files-bundle-jvm.trycmd");
 }

tests/integration/mod.rs

@@ -121,8 +121,30 @@ pub enum ServerBehavior {
     Modern,
 }
 
+#[derive(Debug)]
+pub struct ChunkOptions {
+    chunk_size: usize,
+    missing_chunks: Vec<String>,
+}
+
+impl Default for ChunkOptions {
+    fn default() -> Self {
+        Self {
+            chunk_size: 8388608,
+            missing_chunks: vec![],
+        }
+    }
+}
+
 // Endpoints need to be bound, as they need to live long enough for test to finish
-pub fn mock_common_upload_endpoints(behavior: ServerBehavior) -> Vec<Mock> {
+pub fn mock_common_upload_endpoints(
+    behavior: ServerBehavior,
+    chunk_options: ChunkOptions,
+) -> Vec<Mock> {
+    let ChunkOptions {
+        chunk_size,
+        missing_chunks,
+    } = chunk_options;
     let (accept, release_request_count, assemble_endpoint) = match behavior {
         ServerBehavior::Legacy => (
             "\"release_files\"",
@@ -138,7 +160,7 @@ pub fn mock_common_upload_endpoints(behavior: ServerBehavior) -> Vec<Mock> {
     let chunk_upload_response = format!(
         "{{
             \"url\": \"{}/api/0/organizations/wat-org/chunk-upload/\",
-            \"chunkSize\": 8388608,
+            \"chunkSize\": {chunk_size},
             \"chunksPerRequest\": 64,
             \"maxRequestSize\": 33554432,
             \"concurrency\": 8,
@@ -165,9 +187,12 @@ pub fn mock_common_upload_endpoints(behavior: ServerBehavior) -> Vec<Mock> {
                 .with_response_body("[]"),
         ),
         mock_endpoint(
-            EndpointOptions::new("POST", assemble_endpoint, 200)
-                .with_response_body(r#"{"state":"created","missingChunks":[]}"#),
-        ),
+            EndpointOptions::new("POST", assemble_endpoint, 200).with_response_body(format!(
+                r#"{{"state":"created","missingChunks":{}}}"#,
+                serde_json::to_string(&missing_chunks).unwrap()
+            )),
+        )
+        .expect_at_least(1),
     ]
 }
 

tests/integration/sourcemaps/upload.rs

@@ -1,6 +1,6 @@
 use crate::integration::{
-    assert_endpoints, mock_common_upload_endpoints, mock_endpoint, register_test, EndpointOptions,
-    ServerBehavior,
+    assert_endpoints, mock_common_upload_endpoints, mock_endpoint, register_test, ChunkOptions,
+    EndpointOptions, ServerBehavior,
 };
 
 #[test]
@@ -15,7 +15,7 @@ fn command_sourcemaps_upload() {
 
 #[test]
 fn command_sourcemaps_upload_successfully_upload_file() {
-    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     let _files = mock_endpoint(
         EndpointOptions::new(
             "GET",
@@ -31,7 +31,7 @@ fn command_sourcemaps_upload_successfully_upload_file() {
 
 #[test]
 fn command_sourcemaps_upload_skip_already_uploaded() {
-    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     let _files = mock_endpoint(
         EndpointOptions::new(
             "GET",
@@ -56,7 +56,7 @@ fn command_sourcemaps_upload_skip_already_uploaded() {
 
 #[test]
 fn command_sourcemaps_upload_no_dedupe() {
-    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     let _files = mock_endpoint(
         EndpointOptions::new(
             "GET",
@@ -81,14 +81,22 @@ fn command_sourcemaps_upload_no_dedupe() {
 
 #[test]
 fn command_sourcemaps_upload_modern() {
-    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Modern);
+    let upload_endpoints = mock_common_upload_endpoints(
+        ServerBehavior::Modern,
+        ChunkOptions {
+            missing_chunks: vec!["ec8450a9db19805703a27a2545c18b7b27ba0d7d".to_string()],
+            // Set the chunk size so the bundle will be split into two chunks
+            chunk_size: 512,
+        },
+    );
     register_test("sourcemaps/sourcemaps-upload-modern.trycmd");
     assert_endpoints(&upload_endpoints);
 }
 
 #[test]
 fn command_sourcemaps_upload_empty() {
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Legacy);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Legacy, Default::default());
     let _files = mock_endpoint(
         EndpointOptions::new(
             "GET",
@@ -102,13 +110,20 @@ fn command_sourcemaps_upload_empty() {
 
 #[test]
 fn command_sourcemaps_upload_some_debugids() {
-    let upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Modern);
+    let upload_endpoints = mock_common_upload_endpoints(
+        ServerBehavior::Modern,
+        ChunkOptions {
+            missing_chunks: vec!["5e102ab3da27af9d1095a9c847d4e92a57fe01af".to_string()],
+            chunk_size: 524288,
+        },
+    );
     register_test("sourcemaps/sourcemaps-upload-some-debugids.trycmd");
     assert_endpoints(&upload_endpoints);
 }
 
 #[test]
 fn command_sourcemaps_upload_no_debugids() {
-    let _upload_endpoints = mock_common_upload_endpoints(ServerBehavior::Modern);
+    let _upload_endpoints =
+        mock_common_upload_endpoints(ServerBehavior::Modern, Default::default());
     register_test("sourcemaps/sourcemaps-upload-no-debugids.trycmd");
 }