Skip to content

Commit

Permalink
ref(dif): Genericize try_assemble options parameter
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
szokeasaurusrex committed Dec 4, 2024
1 parent edabdef commit f835979
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 7 deletions.
2 changes: 2 additions & 0 deletions src/utils/chunks/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
//! See `BatchedSliceExt::batches` for more information.
mod types;
mod upload;

pub use types::{Assemblable, Chunked, MissingObjectsInfo};
pub use upload::ChunkOptions;

use std::sync::Arc;
use std::time::Duration;
Expand Down
14 changes: 14 additions & 0 deletions src/utils/chunks/upload.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/// A trait representing options for chunk uploads.
pub trait ChunkOptions {
/// Determines whether we need to strip debug_ids from the requests.
/// When this function returns `true`, the caller is responsible for stripping
/// the debug_ids from the requests, to maintain backwards compatibility with
/// older Sentry servers.
fn should_strip_debug_ids(&self) -> bool;

/// Returns the organization that we are uploading to.
fn org(&self) -> &str;

/// Returns the project that we are uploading to.
fn project(&self) -> &str;
}
28 changes: 21 additions & 7 deletions src/utils/dif_upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ use crate::api::{
use crate::config::Config;
use crate::constants::{DEFAULT_MAX_DIF_SIZE, DEFAULT_MAX_WAIT};
use crate::utils::chunks::{
upload_chunks, Assemblable, BatchedSliceExt, Chunk, Chunked, ItemSize, MissingObjectsInfo,
ASSEMBLE_POLL_INTERVAL,
upload_chunks, Assemblable, BatchedSliceExt, Chunk, ChunkOptions, Chunked, ItemSize,
MissingObjectsInfo, ASSEMBLE_POLL_INTERVAL,
};
use crate::utils::dif::ObjectDifFeatures;
use crate::utils::fs::{get_sha1_checksum, TempDir, TempFile};
Expand Down Expand Up @@ -1229,21 +1229,21 @@ fn create_il2cpp_mappings<'a>(difs: &[DifMatch<'a>]) -> Result<Vec<DifMatch<'a>>
/// missing chunks for convenience.
fn try_assemble<'m, T>(
objects: &'m [Chunked<T>],
options: &DifUpload,
options: &impl ChunkOptions,
) -> Result<MissingObjectsInfo<'m, T>>
where
T: AsRef<[u8]> + Assemblable,
{
let api = Api::current();
let mut request: AssembleDifsRequest<'_> = objects.iter().collect();

if !options.pdbs_allowed {
if options.should_strip_debug_ids() {
request.strip_debug_ids();
}

let response = api
.authenticated()?
.assemble_difs(&options.org, &options.project, &request)?;
let response =
api.authenticated()?
.assemble_difs(options.org(), options.project(), &request)?;

// We map all DIFs by their checksum, so we can access them faster when
// iterating through the server response below. Since the caller will invoke
Expand Down Expand Up @@ -2090,3 +2090,17 @@ impl DifUpload {
true
}
}

impl ChunkOptions for DifUpload {
fn should_strip_debug_ids(&self) -> bool {
self.pdbs_allowed
}

fn org(&self) -> &str {
&self.org
}

fn project(&self) -> &str {
&self.project
}
}

0 comments on commit f835979

Please sign in to comment.