Skip to content

Commit 53ee484

Browse files
ref(dif): Genericize try_assemble options parameter
Introduce new `ChunkOptions` trait which defines an interface for types that store chunk uploading options, and use this trait for the `try_assemble` function's `options` parameter. This change is needed to enable Proguard chunk uploads to use the `try_assemble` function, since Proguard uploads will not be able to use the existing `DifUpload` struct to configure upload options. Refactoring the `DifUpload` struct to also support Proguard uploads would be more complex than having a separate struct for storing Proguard upload options, and having both the `DifUpload` and the Proguard upload struct implement `ChunkOptions`. In the future, we might consider refactoring so that we have one struct that can store upload options for any kind of upload.
1 parent edabdef commit 53ee484

File tree

3 files changed

+42
-7
lines changed

3 files changed

+42
-7
lines changed

src/utils/chunks/mod.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@
66
//! See `BatchedSliceExt::batches` for more information.
77
88
mod types;
9+
mod upload;
910

1011
pub use types::{Assemblable, Chunked, MissingObjectsInfo};
12+
pub use upload::ChunkOptions;
1113

1214
use std::sync::Arc;
1315
use std::time::Duration;

src/utils/chunks/upload.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
/// A trait representing options for chunk uploads.
2+
pub trait ChunkOptions {
3+
/// Determines whether we need to strip debug_ids from the requests.
4+
/// When this function returns `true`, the caller is responsible for stripping
5+
/// the debug_ids from the requests, to maintain backwards compatibility with
6+
/// older Sentry servers.
7+
fn should_strip_debug_ids(&self) -> bool;
8+
9+
/// Returns the organization that we are uploading to.
10+
fn org(&self) -> &str;
11+
12+
/// Returns the project that we are uploading to.
13+
fn project(&self) -> &str;
14+
}

src/utils/dif_upload.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,8 @@ use crate::api::{
3939
use crate::config::Config;
4040
use crate::constants::{DEFAULT_MAX_DIF_SIZE, DEFAULT_MAX_WAIT};
4141
use crate::utils::chunks::{
42-
upload_chunks, Assemblable, BatchedSliceExt, Chunk, Chunked, ItemSize, MissingObjectsInfo,
43-
ASSEMBLE_POLL_INTERVAL,
42+
upload_chunks, Assemblable, BatchedSliceExt, Chunk, ChunkOptions, Chunked, ItemSize,
43+
MissingObjectsInfo, ASSEMBLE_POLL_INTERVAL,
4444
};
4545
use crate::utils::dif::ObjectDifFeatures;
4646
use crate::utils::fs::{get_sha1_checksum, TempDir, TempFile};
@@ -1229,21 +1229,21 @@ fn create_il2cpp_mappings<'a>(difs: &[DifMatch<'a>]) -> Result<Vec<DifMatch<'a>>
12291229
/// missing chunks for convenience.
12301230
fn try_assemble<'m, T>(
12311231
objects: &'m [Chunked<T>],
1232-
options: &DifUpload,
1232+
options: &impl ChunkOptions,
12331233
) -> Result<MissingObjectsInfo<'m, T>>
12341234
where
12351235
T: AsRef<[u8]> + Assemblable,
12361236
{
12371237
let api = Api::current();
12381238
let mut request: AssembleDifsRequest<'_> = objects.iter().collect();
12391239

1240-
if !options.pdbs_allowed {
1240+
if options.should_strip_debug_ids() {
12411241
request.strip_debug_ids();
12421242
}
12431243

1244-
let response = api
1245-
.authenticated()?
1246-
.assemble_difs(&options.org, &options.project, &request)?;
1244+
let response =
1245+
api.authenticated()?
1246+
.assemble_difs(options.org(), options.project(), &request)?;
12471247

12481248
// We map all DIFs by their checksum, so we can access them faster when
12491249
// iterating through the server response below. Since the caller will invoke
@@ -2090,3 +2090,22 @@ impl DifUpload {
20902090
true
20912091
}
20922092
}
2093+
2094+
impl ChunkOptions for DifUpload {
2095+
fn should_strip_debug_ids(&self) -> bool {
2096+
// We need to strip the debug_ids whenever the server does not support
2097+
// chunked uploading of PDBs, to maintain backwards compatibility.
2098+
//
2099+
// See: https://github.com/getsentry/sentry-cli/issues/980
2100+
// See: https://github.com/getsentry/sentry-cli/issues/1056
2101+
!self.pdbs_allowed
2102+
}
2103+
2104+
fn org(&self) -> &str {
2105+
&self.org
2106+
}
2107+
2108+
fn project(&self) -> &str {
2109+
&self.project
2110+
}
2111+
}

0 commit comments

Comments
 (0)