feat: Use checksum to dedupe uploaded release artifacts (#1275)

* feat: Use checksum to dedupe uploaded release artifacts

Author: kamilogorek

src/commands/files/upload.rs

@@ -151,6 +151,7 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
                         ty: SourceFileType::Source,
                         headers: headers.clone(),
                         messages: vec![],
+                        already_uploaded: false,
                     },
                 )
             })

src/commands/sourcemaps/upload.rs

@@ -1,9 +1,11 @@
 use std::path::PathBuf;
+use std::str::FromStr;
 
-use anyhow::Result;
+use anyhow::{format_err, Result};
 use clap::{Arg, ArgMatches, Command};
 use glob::{glob_with, MatchOptions};
 use log::{debug, warn};
+use sha1_smol::Digest;
 
 use crate::api::{Api, NewRelease};
 use crate::config::Config;
@@ -303,6 +305,13 @@ pub fn execute(matches: &ArgMatches) -> Result<()> {
     let api = Api::current();
     let mut processor = SourceMapProcessor::new();
 
+    for artifact in api.list_release_files(&org, Some(&project), &version)? {
+        let checksum = Digest::from_str(&artifact.sha1)
+            .map_err(|_| format_err!("Invalid artifact checksum"))?;
+
+        processor.add_already_uploaded_source(checksum);
+    }
+
     if matches.is_present("bundle") && matches.is_present("bundle_sourcemap") {
         process_sources_from_bundle(matches, &mut processor)?;
     } else {

src/utils/file_upload.rs

@@ -12,14 +12,15 @@ use console::style;
 use parking_lot::RwLock;
 use rayon::prelude::*;
 use rayon::ThreadPoolBuilder;
+use sha1_smol::Digest;
 use symbolic::common::ByteView;
 use symbolic::debuginfo::sourcebundle::{SourceBundleWriter, SourceFileInfo, SourceFileType};
 use url::Url;
 
 use crate::api::{Api, ChunkUploadCapability, ChunkUploadOptions, FileContents, ProgressBarMode};
 use crate::constants::DEFAULT_MAX_WAIT;
 use crate::utils::chunks::{upload_chunks, Chunk, ASSEMBLE_POLL_INTERVAL};
-use crate::utils::fs::{get_sha1_checksums, TempFile};
+use crate::utils::fs::{get_sha1_checksum, get_sha1_checksums, TempFile};
 use crate::utils::progress::{ProgressBar, ProgressStyle};
 
 /// Fallback concurrency for release file uploads.
@@ -56,9 +57,15 @@ pub struct ReleaseFile {
     pub ty: SourceFileType,
     pub headers: Vec<(String, String)>,
     pub messages: Vec<(LogLevel, String)>,
+    pub already_uploaded: bool,
 }
 
 impl ReleaseFile {
+    /// Calculates and returns the SHA1 checksum of the file.
+    pub fn checksum(&self) -> Result<Digest> {
+        get_sha1_checksum(&*self.contents)
+    }
+
     pub fn log(&mut self, level: LogLevel, msg: String) {
         self.messages.push((level, msg));
     }

src/utils/sourcemaps.rs

@@ -9,6 +9,7 @@ use anyhow::{bail, Error, Result};
 use console::style;
 use indicatif::ProgressStyle;
 use log::{debug, info, warn};
+use sha1_smol::Digest;
 use symbolic::debuginfo::sourcebundle::SourceFileType;
 use url::Url;
 
@@ -147,6 +148,7 @@ fn guess_sourcemap_reference(sourcemaps: &HashSet<String>, min_url: &str) -> Res
 
 pub struct SourceMapProcessor {
     pending_sources: HashSet<(String, ReleaseFileMatch)>,
+    already_uploaded_sources: Vec<Digest>,
     sources: ReleaseFiles,
 }
 
@@ -162,6 +164,7 @@ impl SourceMapProcessor {
     pub fn new() -> SourceMapProcessor {
         SourceMapProcessor {
             pending_sources: HashSet::new(),
+            already_uploaded_sources: Vec::new(),
             sources: HashMap::new(),
         }
     }
@@ -172,6 +175,11 @@ impl SourceMapProcessor {
         Ok(())
     }
 
+    /// Adds an already uploaded sources checksum.
+    pub fn add_already_uploaded_source(&mut self, checksum: Digest) {
+        self.already_uploaded_sources.push(checksum);
+    }
+
     fn flush_pending_sources(&mut self) {
         if self.pending_sources.is_empty() {
             return;
@@ -191,6 +199,7 @@ impl SourceMapProcessor {
         );
         for (url, mut file) in self.pending_sources.drain() {
             pb.set_message(&url);
+
             let ty = if sourcemap::is_sourcemap_slice(&file.contents) {
                 SourceFileType::SourceMap
             } else if file
@@ -232,6 +241,7 @@ impl SourceMapProcessor {
                     ty,
                     headers: vec![],
                     messages: vec![],
+                    already_uploaded: false,
                 },
             );
             pb.inc(1);
@@ -268,6 +278,14 @@ impl SourceMapProcessor {
                 sect = Some(source.ty);
             }
 
+            if source.already_uploaded {
+                println!(
+                    "    {}",
+                    style(format!("{} (skipped; already uploaded)", &source.url)).yellow()
+                );
+                continue;
+            }
+
             if source.ty == SourceFileType::MinifiedSource {
                 if let Some(sm_ref) = get_sourcemap_ref(source) {
                     let url = sm_ref.get_url();
@@ -401,6 +419,7 @@ impl SourceMapProcessor {
                     ty: SourceFileType::MinifiedSource,
                     headers: vec![],
                     messages: vec![],
+                    already_uploaded: false,
                 },
             );
 
@@ -418,6 +437,7 @@ impl SourceMapProcessor {
                     ty: SourceFileType::SourceMap,
                     headers: vec![],
                     messages: vec![],
+                    already_uploaded: false,
                 },
             );
         }
@@ -527,9 +547,20 @@ impl SourceMapProcessor {
         Ok(())
     }
 
+    fn flag_uploaded_sources(&mut self) {
+        for source in self.sources.values_mut() {
+            if let Ok(checksum) = &source.checksum() {
+                if self.already_uploaded_sources.contains(checksum) {
+                    source.already_uploaded = true;
+                }
+            }
+        }
+    }
+
     /// Uploads all files
     pub fn upload(&mut self, context: &UploadContext<'_>) -> Result<()> {
         self.flush_pending_sources();
+        self.flag_uploaded_sources();
         let mut uploader = ReleaseFileUpload::new(context);
         uploader.files(&self.sources);
         uploader.upload()?;

tests/integration/_cases/sourcemaps/sourcemaps-upload-help.trycmd

@@ -0,0 +1,96 @@
+```
+$ sentry-cli sourcemaps upload --help
+? success
+sentry-cli[EXE]-sourcemaps-upload 
+Upload sourcemaps for a release.
+
+USAGE:
+    sentry-cli[EXE] sourcemaps upload [OPTIONS] [PATHS]...
+
+ARGS:
+    <PATHS>...    The files to upload.
+
+OPTIONS:
+        --auth-token <AUTH_TOKEN>
+            Use the given Sentry auth token.
+
+        --bundle <BUNDLE>
+            Path to the application bundle (indexed, file, or regular)
+
+        --bundle-sourcemap <BUNDLE_SOURCEMAP>
+            Path to the bundle sourcemap
+
+    -d, --dist <DISTRIBUTION>
+            Optional distribution identifier for the sourcemaps.
+
+    -h, --help
+            Print help information
+
+        --header <KEY:VALUE>
+            Custom headers that should be attached to all requests
+            in key:value format.
+
+    -i, --ignore <IGNORE>
+            Ignores all files and folders matching the given glob
+
+    -I, --ignore-file <IGNORE_FILE>
+            Ignore all files and folders specified in the given ignore file, e.g. .gitignore.
+
+        --log-level <LOG_LEVEL>
+            Set the log output verbosity. [possible values: trace, debug, info, warn, error]
+
+        --no-rewrite
+            Disables rewriting of matching sourcemaps. By default the tool will rewrite sources, so
+            that indexed maps are flattened and missing sources are inlined if possible.
+            This fundamentally changes the upload process to be based on sourcemaps and minified
+            files exclusively and comes in handy for setups like react-native that generate
+            sourcemaps that would otherwise not work for sentry.
+
+        --no-sourcemap-reference
+            Disable emitting of automatic sourcemap references.
+            By default the tool will store a 'Sourcemap' header with minified files so that
+            sourcemaps are located automatically if the tool can detect a link. If this causes
+            issues it can be disabled.
+
+    -o, --org <ORG>
+            The organization slug
+
+    -p, --project <PROJECT>
+            The project slug.
+
+        --quiet
+            Do not print any output while preserving correct exit code. This flag is currently
+            implemented only for selected subcommands. [aliases: silent]
+
+    -r, --release <RELEASE>
+            The release slug.
+
+        --strip-common-prefix
+            Similar to --strip-prefix but strips the most common prefix on all sources references.
+
+        --strip-prefix <PREFIX>
+            Strips the given prefix from all sources references inside the upload sourcemaps (paths
+            used within the sourcemap content, to map minified code to it's original source). Only
+            sources that start with the given prefix will be stripped.
+            This will not modify the uploaded sources paths. To do that, point the upload or
+            upload-sourcemaps command to a more precise directory instead.
+
+    -u, --url-prefix <PREFIX>
+            The URL prefix to prepend to all filenames.
+
+        --url-suffix <SUFFIX>
+            The URL suffix to append to all filenames.
+
+        --validate
+            Enable basic sourcemap validation.
+
+        --wait
+            Wait for the server to fully process uploaded files.
+
+    -x, --ext <EXT>
+            Set the file extensions that are considered for upload. This overrides the default
+            extensions. To add an extension, all default extensions must be repeated. Specify once
+            per extension.
+            Defaults to: `--ext=js --ext=map --ext=jsbundle --ext=bundle`
+
+```

tests/integration/_cases/sourcemaps/sourcemaps-upload-skip-already-uploaded.trycmd

@@ -0,0 +1,20 @@
+```
+$ sentry-cli sourcemaps upload tests/integration/_fixtures/bundle.min.js.map --release=wat-release
+? success
+> Found 1 release file
+> Analyzing 1 sources
+> Rewriting sources
+> Adding source map references
+> Bundled 1 file for upload
+> Uploaded release files to Sentry
+> File upload complete (processing pending on server)
+> Organization: wat-org
+> Project: wat-project
+> Release: wat-release
+> Dist: None
+
+Source Map Upload Report
+  Source Maps
+    ~/bundle.min.js.map (skipped; already uploaded)
+
+```

tests/integration/_cases/sourcemaps/sourcemaps-upload-successfully-upload-file.trycmd

@@ -0,0 +1,20 @@
+```
+$ sentry-cli sourcemaps upload tests/integration/_fixtures/bundle.min.js.map --release=wat-release
+? success
+> Found 1 release file
+> Analyzing 1 sources
+> Rewriting sources
+> Adding source map references
+> Bundled 1 file for upload
+> Uploaded release files to Sentry
+> File upload complete (processing pending on server)
+> Organization: wat-org
+> Project: wat-project
+> Release: wat-release
+> Dist: None
+
+Source Map Upload Report
+  Source Maps
+    ~/bundle.min.js.map
+
+```

tests/integration/_cases/sourcemaps/sourcemaps-upload.trycmd

@@ -0,0 +1,12 @@
+```
+$ sentry-cli sourcemaps upload
+? failed
+error: The following required arguments were not provided:
+    <PATHS>...
+
+USAGE:
+    sentry-cli[EXE] sourcemaps upload <PATHS>...
+
+For more information try --help
+
+```

tests/integration/sourcemaps/mod.rs

@@ -2,6 +2,7 @@ use crate::integration::register_test;
 
 mod explain;
 mod resolve;
+mod upload;
 
 #[test]
 fn command_sourcemaps_help() {