-
-
Notifications
You must be signed in to change notification settings - Fork 240
feat(code-mappings): Add batch splitting for large uploads #3210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: rz/feat/code-mappings-api-integration
Are you sure you want to change the base?
Changes from all commits
59bff1f
ae5351a
4b94685
a6bfd85
1b835b3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4,11 +4,16 @@ use anyhow::{bail, Context as _, Result}; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use clap::{Arg, ArgMatches, Command}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use log::debug; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::api::{Api, BulkCodeMapping, BulkCodeMappingResult, BulkCodeMappingsRequest}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::api::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Api, BulkCodeMapping, BulkCodeMappingResult, BulkCodeMappingsRequest, BulkCodeMappingsResponse, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::config::Config; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::utils::formatting::Table; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::utils::vcs; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Maximum number of mappings the backend accepts per request. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const BATCH_SIZE: usize = 300; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub fn make_command(command: Command) -> Command { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| command | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .about("Upload code mappings for a project from a JSON file. Each mapping pairs a stack trace root (e.g. com/example/module) with the corresponding source path in your repository (e.g. modules/module/src/main/java/com/example/module).") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -144,56 +149,97 @@ pub fn execute(matches: &ArgMatches) -> Result<()> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mapping_count = mappings.len(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let request = BulkCodeMappingsRequest { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| repository: repo_name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default_branch, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mappings, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let batches: Vec<&[BulkCodeMapping]> = mappings.chunks(BATCH_SIZE).collect(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let total_batches = batches.len(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("Uploading {mapping_count} code mapping(s)..."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let api = Api::current(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let response = api | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .authenticated()? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .bulk_upload_code_mappings(&org, &request)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let authenticated = api.authenticated()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut merged = MergedResponse::default(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for (i, batch) in batches.iter().enumerate() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if total_batches > 1 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("Sending batch {}/{total_batches}...", i + 1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let request = BulkCodeMappingsRequest { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| project: project.clone(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| repository: repo_name.clone(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default_branch: default_branch.clone(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mappings: batch.to_vec(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+166
to
+171
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: Please see https://github.com/getsentry/sentry-cli/pull/3209/changes#r2959847064; we can and probably should avoid cloning these strings and the vector. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| match authenticated.bulk_upload_code_mappings(&org, &request) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(response) => merged.add(response), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Err(err) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| merged | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .batch_errors | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .push(format!("Batch {}/{total_batches} failed: {err}", i + 1)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+160
to
+180
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: I think it is a bit cleaner if you separate the merging the response from actually making the bulk requests. You can do this by implementing
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Display error details (successful mappings are summarized in counts only). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print_error_table(&merged.mappings); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for err in &merged.batch_errors { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!("{err}"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| print_results_table(response.mappings); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let total_errors = merged.errors + merged.batch_errors.len() as u64; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Created: {}, Updated: {}, Errors: {}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.created, response.updated, response.errors | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Created: {}, Updated: {}, Errors: {total_errors}", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| merged.created, merged.updated | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if response.errors > 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bail!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "{} mapping(s) failed to upload. See errors above.", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.errors | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if total_errors > 0 { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bail!("{total_errors} error(s) during upload. See details above."); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
cursor[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn print_results_table(mappings: Vec<BulkCodeMappingResult>) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn print_error_table(mappings: &[BulkCodeMappingResult]) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let error_mappings: Vec<_> = mappings.iter().filter(|r| r.status == "error").collect(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if error_mappings.is_empty() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut table = Table::new(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| table | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .title_row() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add("Stack Root") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add("Source Root") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add("Status"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add("Detail"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for result in mappings { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let status = match result.detail { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Some(detail) if result.status == "error" => format!("error: {detail}"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _ => result.status, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for result in &error_mappings { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+203
to
+216
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: We can avoid allocating a vector for the
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let detail = result.detail.as_deref().unwrap_or("unknown error"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| table | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add_row() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add(&result.stack_root) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add(&result.source_root) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add(&status); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .add(detail); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| table.print(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| println!(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Default)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| struct MergedResponse { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| created: u64, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| updated: u64, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| errors: u64, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mappings: Vec<BulkCodeMappingResult>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| batch_errors: Vec<String>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl MergedResponse { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fn add(&mut self, response: BulkCodeMappingsResponse) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.created += response.created; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.updated += response.updated; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.errors += response.errors; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.mappings.extend(response.mappings); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+238
to
+245
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: By implementing
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| ``` | ||
| $ sentry-cli code-mappings upload tests/integration/_fixtures/code_mappings/mappings.json --org wat-org --project wat-project --repo owner/repo --default-branch main | ||
| ? failed | ||
| Uploading 2 code mapping(s)... | ||
| +------------------+---------------------------------------------+-------------------+ | ||
| | Stack Root | Source Root | Detail | | ||
| +------------------+---------------------------------------------+-------------------+ | ||
| | com/example/maps | modules/maps/src/main/java/com/example/maps | duplicate mapping | | ||
| +------------------+---------------------------------------------+-------------------+ | ||
|
|
||
| Created: 1, Updated: 0, Errors: 1 | ||
| error: 1 error(s) during upload. See details above. | ||
|
|
||
| Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output. | ||
| Please attach the full debug log to all bug reports. | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| ``` | ||
| $ sentry-cli code-mappings upload tests/integration/_fixtures/code_mappings/mappings.json --org wat-org --project wat-project --repo owner/repo --default-branch main | ||
| ? failed | ||
| Uploading 2 code mapping(s)... | ||
| +------------------+---------------------------------------------+-------------------+ | ||
| | Stack Root | Source Root | Detail | | ||
| +------------------+---------------------------------------------+-------------------+ | ||
| | com/example/maps | modules/maps/src/main/java/com/example/maps | duplicate mapping | | ||
| +------------------+---------------------------------------------+-------------------+ | ||
|
|
||
| Created: 1, Updated: 0, Errors: 1 | ||
| error: 1 error(s) during upload. See details above. | ||
|
|
||
| Add --log-level=[info|debug] or export SENTRY_LOG_LEVEL=[info|debug] to see more output. | ||
| Please attach the full debug log to all bug reports. | ||
|
|
||
| ``` |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "created": 1, | ||
| "updated": 0, | ||
| "errors": 1, | ||
| "mappings": [ | ||
| {"stackRoot": "com/example/core", "sourceRoot": "modules/core/src/main/java/com/example/core", "status": "created"}, | ||
| {"stackRoot": "com/example/maps", "sourceRoot": "modules/maps/src/main/java/com/example/maps", "status": "error", "detail": "duplicate mapping"} | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,9 @@ | ||
| { | ||
| "created": 1, | ||
| "updated": 0, | ||
| "errors": 1, | ||
| "mappings": [ | ||
| {"stackRoot": "com/example/core", "sourceRoot": "modules/core/src/main/java/com/example/core", "status": "created"}, | ||
| {"stackRoot": "com/example/maps", "sourceRoot": "modules/maps/src/main/java/com/example/maps", "status": "error", "detail": "duplicate mapping"} | ||
| ] | ||
| } |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,4 +1,6 @@ | ||||||
| use crate::integration::{MockEndpointBuilder, TestManager}; | ||||||
| use std::sync::atomic::{AtomicU16, Ordering}; | ||||||
|
|
||||||
| use crate::integration::{AssertCommand, MockEndpointBuilder, TestManager}; | ||||||
|
|
||||||
| #[test] | ||||||
| fn command_code_mappings_upload() { | ||||||
|
|
@@ -10,3 +12,84 @@ fn command_code_mappings_upload() { | |||||
| .register_trycmd_test("code_mappings/code-mappings-upload.trycmd") | ||||||
| .with_default_token(); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn command_code_mappings_upload_partial_error() { | ||||||
| TestManager::new() | ||||||
| .mock_endpoint( | ||||||
| MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/code-mappings/bulk/") | ||||||
| .with_response_file("code_mappings/post-bulk-partial-error.json"), | ||||||
| ) | ||||||
| .register_trycmd_test("code_mappings/code-mappings-upload-partial-error.trycmd") | ||||||
| .with_default_token(); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn command_code_mappings_upload_207_partial_error() { | ||||||
| TestManager::new() | ||||||
| .mock_endpoint( | ||||||
| MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/code-mappings/bulk/") | ||||||
| .with_status(207) | ||||||
| .with_response_file("code_mappings/post-bulk-207-partial-error.json"), | ||||||
| ) | ||||||
| .register_trycmd_test("code_mappings/code-mappings-upload-207-partial-error.trycmd") | ||||||
| .with_default_token(); | ||||||
| } | ||||||
|
|
||||||
| #[test] | ||||||
| fn command_code_mappings_upload_batches() { | ||||||
| // Generate a fixture with 301 mappings to force 2 batches (300 + 1). | ||||||
| let mut mappings = Vec::with_capacity(301); | ||||||
| for i in 0..301 { | ||||||
| mappings.push(serde_json::json!({ | ||||||
| "stackRoot": format!("com/example/m{i}"), | ||||||
| "sourceRoot": format!("modules/m{i}/src/main/java/com/example/m{i}"), | ||||||
| })); | ||||||
| } | ||||||
| let fixture = tempfile::NamedTempFile::new().expect("failed to create temp file"); | ||||||
| serde_json::to_writer(&fixture, &mappings).expect("failed to write fixture"); | ||||||
|
|
||||||
| let call_count = AtomicU16::new(0); | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. l: 8 bits is enough
Suggested change
|
||||||
|
|
||||||
| TestManager::new() | ||||||
| .mock_endpoint( | ||||||
| MockEndpointBuilder::new("POST", "/api/0/organizations/wat-org/code-mappings/bulk/") | ||||||
| .expect(2) | ||||||
| .with_response_fn(move |_request| { | ||||||
| let n = call_count.fetch_add(1, Ordering::Relaxed); | ||||||
| // Return appropriate counts per batch | ||||||
| let (created, mapping_count) = if n == 0 { (300, 300) } else { (1, 1) }; | ||||||
| let mut batch_mappings = Vec::new(); | ||||||
| for i in 0..mapping_count { | ||||||
| let idx = n as usize * 300 + i; | ||||||
| batch_mappings.push(serde_json::json!({ | ||||||
| "stackRoot": format!("com/example/m{idx}"), | ||||||
| "sourceRoot": format!("modules/m{idx}/src/main/java/com/example/m{idx}"), | ||||||
| "status": "created", | ||||||
| })); | ||||||
| } | ||||||
| serde_json::to_vec(&serde_json::json!({ | ||||||
| "created": created, | ||||||
| "updated": 0, | ||||||
| "errors": 0, | ||||||
| "mappings": batch_mappings, | ||||||
| })) | ||||||
| .expect("failed to serialize response") | ||||||
| }), | ||||||
| ) | ||||||
| .assert_cmd([ | ||||||
| "code-mappings", | ||||||
| "upload", | ||||||
| fixture.path().to_str().expect("valid utf-8 path"), | ||||||
| "--org", | ||||||
| "wat-org", | ||||||
| "--project", | ||||||
| "wat-project", | ||||||
| "--repo", | ||||||
| "owner/repo", | ||||||
| "--default-branch", | ||||||
| "main", | ||||||
| ]) | ||||||
| .with_default_token() | ||||||
| .run_and_assert(AssertCommand::Success); | ||||||
| } | ||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l: You can avoid allocating a
Vecfor the chunks pretty easily. Here, you compute the total batches usingdiv_ceil, and in the loop, you iterate the batches directly, computing them lazily on each iteration.