From 614330657fd496caae34cfd86ec289bdc16ef0b1 Mon Sep 17 00:00:00 2001 From: Matt Hammerly Date: Thu, 13 Jun 2024 15:53:15 -0700 Subject: [PATCH 1/6] move insert logic into new Insertable model trait --- src/report/sqlite/models.rs | 540 +++++++++++++++++++++++++++- src/report/sqlite/report_builder.rs | 104 ++---- 2 files changed, 564 insertions(+), 80 deletions(-) diff --git a/src/report/sqlite/models.rs b/src/report/sqlite/models.rs index f0c12ce..f5ccb90 100644 --- a/src/report/sqlite/models.rs +++ b/src/report/sqlite/models.rs @@ -5,13 +5,75 @@ * `ToSql`/`FromSql` allow enums to be used as model fields. * `TryFrom<&rusqlite::Row>` allows models to be automatically constructed * from query results (provided the query's column names are - * named appropriately). + * named appropriately). [`Insertable`] takes care of the boilerplate to + * insert a model into the database when provided a few constants for each + * model. */ use rusqlite::types::{FromSql, FromSqlError, FromSqlResult, ToSql, ToSqlOutput, ValueRef}; use super::super::models::*; -use crate::parsers::json::JsonVal; +use crate::{error::Result, parsers::json::JsonVal}; + +/// Takes care of the boilerplate to insert a model into the database. +/// Implementers must provide four things: +/// - `const INSERT_QUERY_PRELUDE: &'static str;`: the "INSERT INTO ... (...) +/// VALUES " bit of a query +/// - `const INSERT_PLACEHOLDER: &'static str;`: a tuple with the appropriate +/// number of `?`s to represent a single record. Placed after the "VALUES" +/// keyword in an insert query. +/// - `fn param_bindings(&self) -> [&dyn rusqlite::ToSql; FIELD_COUNT]`: a +/// function which returns an array of `ToSql` trait objects that should bind +/// to each of the `?`s in `INSERT_PLACEHOLDER`. +/// - `fn assign_id(&mut self)`: a function which generates and sets an +/// appropriate ID for the model. +/// +/// Example: +/// ``` +/// # use codecov_rs::report::sqlite::Insertable; +/// struct File { +/// id: i64, +/// path: String, +/// } +/// +/// impl Insertable<2> for File { +/// const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO file (id, path) VALUES "; +/// const INSERT_PLACEHOLDER: &'static str = "(?, ?)"; +/// +/// fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 2] { +/// [ +/// &self.id as &dyn rusqlite::ToSql, +/// &self.path as &dyn rusqlite::ToSql, +/// ] +/// } +/// +/// fn assign_id(&mut self) { +/// self.id = seahash::hash(self.path.as_bytes()) as i64; +/// } +/// } +/// ``` +/// +/// IDs are not assigned automatically; assign your own to models before you +/// insert them. +pub trait Insertable { + const INSERT_QUERY_PRELUDE: &'static str; + + const INSERT_PLACEHOLDER: &'static str; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; FIELD_COUNT]; + + fn assign_id(&mut self); + + fn insert(model: &Self, conn: &rusqlite::Connection) -> Result<()> { + let mut stmt = conn.prepare_cached( + // Maybe turn this in to a lazily-initialized static + format!("{}{}", Self::INSERT_QUERY_PRELUDE, Self::INSERT_PLACEHOLDER).as_str(), + )?; + stmt.execute(rusqlite::params_from_iter(model.param_bindings()))?; + + Ok(()) + } +} /// Can't implement foreign traits (`ToSql`/`FromSql`) on foreign types /// (`serde_json::Value`) so this helper function fills in. @@ -91,6 +153,22 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for SourceFile { } } +impl Insertable<2> for SourceFile { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO source_file (id, path) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 2] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.path as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = seahash::hash(self.path.as_bytes()) as i64; + } +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for CoverageSample { type Error = rusqlite::Error; @@ -107,6 +185,27 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for CoverageSample { } } +impl Insertable<7> for CoverageSample { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO coverage_sample (id, source_file_id, line_no, coverage_type, hits, hit_branches, total_branches) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 7] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.source_file_id as &dyn rusqlite::ToSql, + &self.line_no as &dyn rusqlite::ToSql, + &self.coverage_type as &dyn rusqlite::ToSql, + &self.hits as &dyn rusqlite::ToSql, + &self.hit_branches as &dyn rusqlite::ToSql, + &self.total_branches as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = uuid::Uuid::new_v4(); + } +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for BranchesData { type Error = rusqlite::Error; @@ -121,6 +220,27 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for BranchesData { }) } } + +impl Insertable<6> for BranchesData { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO branches_data (id, source_file_id, sample_id, hits, branch_format, branch) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 6] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.source_file_id as &dyn rusqlite::ToSql, + &self.sample_id as &dyn rusqlite::ToSql, + &self.hits as &dyn rusqlite::ToSql, + &self.branch_format as &dyn rusqlite::ToSql, + &self.branch as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = uuid::Uuid::new_v4(); + } +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for MethodData { type Error = rusqlite::Error; @@ -138,6 +258,28 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for MethodData { } } +impl Insertable<8> for MethodData { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO method_data (id, source_file_id, sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 8] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.source_file_id as &dyn rusqlite::ToSql, + &self.sample_id as &dyn rusqlite::ToSql, + &self.line_no as &dyn rusqlite::ToSql, + &self.hit_branches as &dyn rusqlite::ToSql, + &self.total_branches as &dyn rusqlite::ToSql, + &self.hit_complexity_paths as &dyn rusqlite::ToSql, + &self.total_complexity as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = uuid::Uuid::new_v4(); + } +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for SpanData { type Error = rusqlite::Error; @@ -155,6 +297,28 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for SpanData { } } +impl Insertable<8> for SpanData { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO span_data (id, source_file_id, sample_id, hits, start_line, start_col, end_line, end_col) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 8] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.source_file_id as &dyn rusqlite::ToSql, + &self.sample_id as &dyn rusqlite::ToSql, + &self.hits as &dyn rusqlite::ToSql, + &self.start_line as &dyn rusqlite::ToSql, + &self.start_col as &dyn rusqlite::ToSql, + &self.end_line as &dyn rusqlite::ToSql, + &self.end_col as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = uuid::Uuid::new_v4(); + } +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for ContextAssoc { type Error = rusqlite::Error; @@ -169,6 +333,24 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for ContextAssoc { } } +impl Insertable<5> for ContextAssoc { + const INSERT_QUERY_PRELUDE: &'static str = + "INSERT INTO context_assoc (context_id, sample_id, branch_id, method_id, span_id) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 5] { + [ + &self.context_id as &dyn rusqlite::ToSql, + &self.sample_id as &dyn rusqlite::ToSql, + &self.branch_id as &dyn rusqlite::ToSql, + &self.method_id as &dyn rusqlite::ToSql, + &self.span_id as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) {} +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for Context { type Error = rusqlite::Error; @@ -181,6 +363,24 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for Context { } } +impl Insertable<3> for Context { + const INSERT_QUERY_PRELUDE: &'static str = + "INSERT INTO context (id, context_type, name) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 3] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.context_type as &dyn rusqlite::ToSql, + &self.name as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = seahash::hash(self.name.as_bytes()) as i64; + } +} + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for UploadDetails { type Error = rusqlite::Error; @@ -245,3 +445,339 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for ReportTotals { }) } } + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + + use super::{ + super::{super::Report, SqliteReport}, + *, + }; + + #[derive(PartialEq, Debug)] + struct TestModel { + id: i64, + data: String, + } + + impl Insertable<2> for TestModel { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO test (id, data) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?)"; + + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 2] { + [ + &self.id as &dyn rusqlite::ToSql, + &self.data as &dyn rusqlite::ToSql, + ] + } + + fn assign_id(&mut self) { + self.id = seahash::hash(self.data.as_bytes()) as i64; + } + } + + impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for TestModel { + type Error = rusqlite::Error; + + fn try_from(row: &'a ::rusqlite::Row) -> Result { + Ok(Self { + id: row.get(row.as_ref().column_index("id")?)?, + data: row.get(row.as_ref().column_index("data")?)?, + }) + } + } + + struct Ctx { + _temp_dir: TempDir, + report: SqliteReport, + } + + fn setup() -> Ctx { + let temp_dir = TempDir::new().ok().unwrap(); + let db_file = temp_dir.path().join("db.sqlite"); + let report = SqliteReport::new(db_file).unwrap(); + + report + .conn + .execute( + "CREATE TABLE test (id INTEGER PRIMARY KEY, data VARCHAR)", + [], + ) + .unwrap(); + + Ctx { + _temp_dir: temp_dir, + report, + } + } + + fn list_test_models(report: &SqliteReport) -> Vec { + let mut stmt = report + .conn + .prepare_cached("SELECT id, data FROM test ORDER BY id ASC") + .unwrap(); + let models = stmt + .query_map([], |row| row.try_into()) + .unwrap() + .collect::>>() + .unwrap(); + + models + } + + #[test] + fn test_test_model_single_insert() { + let ctx = setup(); + + let model = TestModel { + id: 5, + data: "foo".to_string(), + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let test_models = list_test_models(&ctx.report); + assert_eq!(test_models, vec![model]); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: test.id'" + ); + } + + #[test] + fn test_source_file_single_insert() { + let ctx = setup(); + + let model = SourceFile { + id: 0, + path: "src/report/report.rs".to_string(), + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let files = ctx.report.list_files().unwrap(); + assert_eq!(files, vec![model]); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: source_file.id'" + ); + } + + #[test] + fn test_context_single_insert() { + let ctx = setup(); + + let model = Context { + id: 0, + context_type: ContextType::Upload, + name: "test_upload".to_string(), + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let contexts = ctx.report.list_contexts().unwrap(); + assert_eq!(contexts, vec![model]); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: context.id'" + ); + } + + #[test] + fn test_context_assoc_single_insert() { + let ctx = setup(); + + let model = ContextAssoc { + context_id: 0, + sample_id: Some(uuid::Uuid::new_v4()), + ..Default::default() + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let assoc: ContextAssoc = ctx + .report + .conn + .query_row( + "SELECT context_id, sample_id, branch_id, method_id, span_id FROM context_assoc", + [], + |row| row.try_into(), + ) + .unwrap(); + assert_eq!(assoc, model); + + let duplicate_result = >::insert(&model, &ctx.report.conn); + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: context_assoc.context_id, context_assoc.sample_id'" + ); + } + + #[test] + fn test_coverage_sample_single_insert() { + let ctx = setup(); + + >::insert( + &SourceFile { + id: 0, + ..Default::default() + }, + &ctx.report.conn, + ) + .unwrap(); + + let model = CoverageSample { + id: uuid::Uuid::new_v4(), + source_file_id: 0, + ..Default::default() + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let samples = ctx.report.list_coverage_samples().unwrap(); + assert_eq!(samples, vec![model]); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: coverage_sample.id'" + ); + } + + #[test] + fn test_branches_data_single_insert() { + let ctx = setup(); + + >::insert( + &SourceFile { + id: 0, + ..Default::default() + }, + &ctx.report.conn, + ) + .unwrap(); + + let sample_id = uuid::Uuid::new_v4(); + >::insert( + &CoverageSample { + id: sample_id, + source_file_id: 0, + ..Default::default() + }, + &ctx.report.conn, + ) + .unwrap(); + + let model = BranchesData { + id: uuid::Uuid::new_v4(), + sample_id, + source_file_id: 0, + ..Default::default() + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let branch: BranchesData = ctx.report + .conn + .query_row( + "SELECT id, source_file_id, sample_id, hits, branch_format, branch FROM branches_data", + [], + |row| row.try_into(), + ).unwrap(); + assert_eq!(branch, model); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: branches_data.id'" + ); + } + + #[test] + fn test_method_data_single_insert() { + let ctx = setup(); + + >::insert( + &SourceFile { + id: 0, + ..Default::default() + }, + &ctx.report.conn, + ) + .unwrap(); + + let model = MethodData { + id: uuid::Uuid::new_v4(), + source_file_id: 0, + ..Default::default() + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let method: MethodData = ctx.report + .conn + .query_row( + "SELECT id, source_file_id, sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity FROM method_data", + [], + |row| row.try_into(), + ).unwrap(); + assert_eq!(method, model); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: method_data.id'" + ); + } + + #[test] + fn test_span_data_single_insert() { + let ctx = setup(); + + >::insert( + &SourceFile { + id: 0, + ..Default::default() + }, + &ctx.report.conn, + ) + .unwrap(); + + let model = SpanData { + id: uuid::Uuid::new_v4(), + source_file_id: 0, + ..Default::default() + }; + + >::insert(&model, &ctx.report.conn).unwrap(); + let duplicate_result = >::insert(&model, &ctx.report.conn); + + let branch: SpanData = ctx.report + .conn + .query_row( + "SELECT id, source_file_id, sample_id, hits, start_line, start_col, end_line, end_col FROM span_data", + [], + |row| row.try_into(), + ).unwrap(); + assert_eq!(branch, model); + + let error = duplicate_result.unwrap_err(); + assert_eq!( + error.to_string(), + "sqlite failure: 'UNIQUE constraint failed: span_data.id'" + ); + } +} diff --git a/src/report/sqlite/report_builder.rs b/src/report/sqlite/report_builder.rs index 1662f60..30c75c1 100644 --- a/src/report/sqlite/report_builder.rs +++ b/src/report/sqlite/report_builder.rs @@ -1,9 +1,8 @@ use std::path::{Path, PathBuf}; use rusqlite::{Connection, Transaction}; -use uuid::Uuid; -use super::{open_database, SqliteReport}; +use super::{models::Insertable, open_database, SqliteReport}; use crate::{ error::{CodecovError, Result}, report::{models, ReportBuilder}, @@ -184,15 +183,13 @@ impl ReportBuilder for SqliteReportBuilder { impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { fn insert_file(&mut self, path: String) -> Result { - let mut stmt = self.conn.prepare_cached( - "INSERT INTO source_file (id, path) VALUES (?1, ?2) RETURNING id, path", - )?; - - Ok( - stmt.query_row((seahash::hash(path.as_bytes()) as i64, path), |row| { - row.try_into() - })?, - ) + let mut model = models::SourceFile { + path, + ..Default::default() + }; + model.assign_id(); + >::insert(&model, &self.conn)?; + Ok(model) } fn insert_context( @@ -200,33 +197,22 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { context_type: models::ContextType, name: &str, ) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO context (id, context_type, name) VALUES (?1, ?2, ?3) RETURNING id, context_type, name")?; - Ok(stmt.query_row( - ( - seahash::hash(name.as_bytes()) as i64, - context_type.to_string(), - name, - ), - |row| row.try_into(), - )?) + let mut model = models::Context { + context_type, + name: name.to_string(), + ..Default::default() + }; + model.assign_id(); + >::insert(&model, &self.conn)?; + Ok(model) } fn insert_coverage_sample( &mut self, mut sample: models::CoverageSample, ) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO coverage_sample (id, source_file_id, line_no, coverage_type, hits, hit_branches, total_branches) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7)")?; - sample.id = Uuid::new_v4(); - let _ = stmt.execute(( - sample.id, - sample.source_file_id, - sample.line_no, - sample.coverage_type, - sample.hits, - sample.hit_branches, - sample.total_branches, - ))?; - + sample.assign_id(); + >::insert(&sample, &self.conn)?; Ok(sample) } @@ -234,51 +220,20 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { &mut self, mut branch: models::BranchesData, ) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO branches_data (id, source_file_id, sample_id, hits, branch_format, branch) VALUES (?1, ?2, ?3, ?4, ?5, ?6)")?; - - branch.id = Uuid::new_v4(); - let _ = stmt.execute(( - branch.id, - branch.source_file_id, - branch.sample_id, - branch.hits, - branch.branch_format, - &branch.branch, - ))?; + branch.assign_id(); + >::insert(&branch, &self.conn)?; Ok(branch) } fn insert_method_data(&mut self, mut method: models::MethodData) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO method_data (id, source_file_id, sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)")?; - method.id = Uuid::new_v4(); - - let _ = stmt.execute(( - method.id, - method.source_file_id, - method.sample_id, - method.line_no, - method.hit_branches, - method.total_branches, - method.hit_complexity_paths, - method.total_complexity, - ))?; + method.assign_id(); + >::insert(&method, &self.conn)?; Ok(method) } fn insert_span_data(&mut self, mut span: models::SpanData) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO span_data (id, source_file_id, sample_id, hits, start_line, start_col, end_line, end_col) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8)")?; - span.id = Uuid::new_v4(); - - let _ = stmt.execute(( - span.id, - span.source_file_id, - span.sample_id, - span.hits, - span.start_line, - span.start_col, - span.end_line, - span.end_col, - ))?; + span.assign_id(); + >::insert(&span, &self.conn)?; Ok(span) } @@ -286,15 +241,7 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { &mut self, assoc: models::ContextAssoc, ) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO context_assoc (context_id, sample_id, branch_id, method_id, span_id) VALUES (?1, ?2, ?3, ?4, ?5)")?; - - let _ = stmt.execute(( - assoc.context_id, - assoc.sample_id, - assoc.branch_id, - assoc.method_id, - assoc.span_id, - ))?; + >::insert(&assoc, &self.conn)?; Ok(assoc) } @@ -339,6 +286,7 @@ mod tests { use rusqlite_migration::SchemaVersion; use serde_json::json; use tempfile::TempDir; + use uuid::Uuid; use super::*; use crate::report::Report; From 77b522f1bee753cf5d1cdd6e848c2cd0adbb1596 Mon Sep 17 00:00:00 2001 From: Matt Hammerly Date: Fri, 26 Jul 2024 13:24:43 -0700 Subject: [PATCH 2/6] replace UUIDs with joint i64 PKs in sqlite schema --- Cargo.lock | 48 ++- Cargo.toml | 4 +- migrations/01-init/up.sql | 149 ++++--- src/parsers/pyreport/mod.rs | 26 +- src/parsers/pyreport/report_json.rs | 376 +++++------------- src/parsers/pyreport/utils.rs | 81 ++-- src/report/mod.rs | 23 +- src/report/models.rs | 192 +++++---- src/report/pyreport/chunks.rs | 173 ++++---- .../pyreport/queries/chunks_file_header.sql | 2 - .../pyreport/queries/files_to_report_json.sql | 28 +- .../pyreport/queries/samples_to_chunks.sql | 81 +--- .../queries/sessions_to_report_json.sql | 48 +-- src/report/pyreport/report_json.rs | 189 ++++----- src/report/sqlite/mod.rs | 1 - src/report/sqlite/models.rs | 317 ++++++++------- src/report/sqlite/queries/totals.sql | 7 +- src/report/sqlite/report.rs | 104 +++-- src/report/sqlite/report_builder.rs | 302 ++++++++------ tests/test_pyreport_shim.rs | 160 ++++---- 20 files changed, 1148 insertions(+), 1163 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index aaaf857..f65d071 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -52,6 +52,7 @@ dependencies = [ "lazy_static", "memmap2", "mockall", + "rand", "rusqlite", "rusqlite_migration", "seahash", @@ -60,7 +61,6 @@ dependencies = [ "strum_macros", "tempfile", "thiserror", - "uuid", "winnow", ] @@ -254,6 +254,12 @@ version = "0.3.30" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "d231b230927b5e4ad203db57bbcbee2802f6bce620b1e4a9024a07d94e2907ec" +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + [[package]] name = "predicates" version = "3.1.0" @@ -298,6 +304,36 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "rusqlite" version = "0.30.0" @@ -310,7 +346,6 @@ dependencies = [ "hashlink", "libsqlite3-sys", "smallvec", - "uuid", ] [[package]] @@ -466,15 +501,6 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" -[[package]] -name = "uuid" -version = "1.8.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a183cf7feeba97b4dd1c0d46788634f6221d87fa961b305bed08c851829efcc0" -dependencies = [ - "getrandom", -] - [[package]] name = "vcpkg" version = "0.2.15" diff --git a/Cargo.toml b/Cargo.toml index 6670fb6..0064659 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,9 +4,9 @@ version = "0.1.0" edition = "2021" [dependencies] -rusqlite = { version = "0.30.0", features = ["bundled", "uuid"] } +rusqlite = { version = "0.30.0", features = ["bundled"] } rusqlite_migration = { version = "1.1.0", features = ["from-directory"] } -uuid = { version = "1.8.0", features = ["v4"] } +rand = "0.8.5" # SeaHash chosen due to: # - widely used diff --git a/migrations/01-init/up.sql b/migrations/01-init/up.sql index 06fba59..05bf07a 100644 --- a/migrations/01-init/up.sql +++ b/migrations/01-init/up.sql @@ -1,76 +1,135 @@ +-- See `src/report/models.rs` for complete, up-to-date schema documentation. + +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` CREATE TABLE source_file ( + -- This should be set to the hash of the `path` column so that we can + -- distribute processing across multiple different hosts and they will + -- all come up with the same ID. id INTEGER PRIMARY KEY, + path VARCHAR NOT NULL -) WITHOUT ROWID; +); + +-- TODO: Allow distinguishing between raw reports within a single upload +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` +CREATE TABLE raw_upload ( + -- This should be set to a random 64-bit integer so that we can + -- distribute processing across multiple different hosts and they will + -- not fight over autoincrementing ID values. + id INTEGER PRIMARY KEY, + + timestamp INTEGER, + raw_upload_url VARCHAR, + flags VARCHAR, -- JSON + provider VARCHAR, + build VARCHAR, + name VARCHAR, + job_name VARCHAR, + ci_run_url VARCHAR, + state VARCHAR, + env VARCHAR, + session_type VARCHAR, + session_extras VARCHAR -- JSON, +); + +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` +CREATE TABLE context ( + -- This should be set to the hash of the `name` column so that we can + -- distribute processing across multiple different hosts and they will + -- all come up with the same ID. + id INTEGER PRIMARY KEY, + + context_type VARCHAR NOT NULL, + name VARCHAR NOT NULL +); + +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` +CREATE TABLE context_assoc ( + context_id INTEGER REFERENCES context(id) NOT NULL, + + raw_upload_id INTEGER NOT NULL, + local_sample_id INTEGER, + local_span_id INTEGER, + -- TODO: Figure out how to re-enable these +-- FOREIGN KEY (raw_upload_id, local_sample_id) REFERENCES coverage_sample(raw_upload_id, local_sample_id), +-- FOREIGN KEY (raw_upload_id, local_span_id) REFERENCES span_data(raw_upload_id, local_span_id), + + PRIMARY KEY (context_id, raw_upload_id, local_sample_id, local_span_id) +); + +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` CREATE TABLE coverage_sample ( - id BLOB PRIMARY KEY, + raw_upload_id INTEGER REFERENCES raw_upload(id) NOT NULL, + + -- This should be an application-managed auto-incremented integer. + local_sample_id INTEGER NOT NULL, + source_file_id INTEGER REFERENCES source_file(id) NOT NULL, line_no INTEGER NOT NULL, + coverage_type VARCHAR NOT NULL, hits INTEGER, hit_branches INTEGER, - total_branches INTEGER -) WITHOUT ROWID; + total_branches INTEGER, + + PRIMARY KEY (raw_upload_id, local_sample_id) +); +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` CREATE TABLE branches_data ( - id BLOB PRIMARY KEY, + raw_upload_id INTEGER REFERENCES raw_upload(id) NOT NULL, + local_sample_id INTEGER NOT NULL, + + -- This should be an application-managed auto-incremented integer. + local_branch_id INTEGER NOT NULL, + source_file_id INTEGER REFERENCES source_file(id) NOT NULL, - sample_id BLOB REFERENCES coverage_sample(id) NOT NULL, + hits INTEGER NOT NULL, branch_format VARCHAR NOT NULL, - branch VARCHAR NOT NULL -) WITHOUT ROWID; + branch VARCHAR NOT NULL, + + FOREIGN KEY (raw_upload_id, local_sample_id) REFERENCES coverage_sample(raw_upload_id, local_sample_id), + PRIMARY KEY (raw_upload_id, local_branch_id) +); +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` CREATE TABLE method_data ( - id BLOB PRIMARY KEY, + raw_upload_id INTEGER REFERENCES raw_upload(id) NOT NULL, + local_sample_id INTEGER NOT NULL, + + -- This should be an application-managed auto-incremented integer. + local_method_id INTEGER NOT NULL, + source_file_id INTEGER REFERENCES source_file(id) NOT NULL, - sample_id BLOB REFERENCES coverage_sample(id), line_no INTEGER, + hit_branches INTEGER, total_branches INTEGER, hit_complexity_paths INTEGER, - total_complexity INTEGER -) WITHOUT ROWID; + total_complexity INTEGER, + FOREIGN KEY (raw_upload_id, local_sample_id) REFERENCES coverage_sample(raw_upload_id, local_sample_id), + PRIMARY KEY (raw_upload_id, local_method_id) +); + +-- TODO: Measure size/perf impact of making this table `WITHOUT ROWID` CREATE TABLE span_data ( - id BLOB PRIMARY KEY, + raw_upload_id INTEGER REFERENCES raw_upload(id) NOT NULL, + local_sample_id INTEGER, + + -- This should be an application-managed auto-incremented integer. + local_span_id INTEGER NOT NULL, + source_file_id INTEGER REFERENCES source_file(id) NOT NULL, - sample_id BLOB REFERENCES coverage_sample(id), + hits INTEGER NOT NULL, start_line INTEGER, start_col INTEGER, end_line INTEGER, - end_col INTEGER -) WITHOUT ROWID; - -CREATE TABLE context ( - id INTEGER PRIMARY KEY, - context_type VARCHAR NOT NULL, - name VARCHAR NOT NULL -); + end_col INTEGER, -CREATE TABLE context_assoc ( - context_id INTEGER NOT NULL, - sample_id BLOB, - branch_id BLOB, - method_id BLOB, - span_id BLOB, - PRIMARY KEY(context_id, sample_id) -) WITHOUT ROWID; - -CREATE TABLE upload_details ( - context_id INTEGER REFERENCES context(id) NOT NULL, - timestamp INTEGER, - raw_upload_url VARCHAR, - flags VARCHAR, -- JSON - provider VARCHAR, - build VARCHAR, - name VARCHAR, - job_name VARCHAR, - ci_run_url VARCHAR, - state VARCHAR, - env VARCHAR, - session_type VARCHAR, - session_extras VARCHAR -- JSON, + FOREIGN KEY (raw_upload_id, local_sample_id) REFERENCES coverage_sample(raw_upload_id, local_sample_id), + PRIMARY KEY (raw_upload_id, local_span_id) ); diff --git a/src/parsers/pyreport/mod.rs b/src/parsers/pyreport/mod.rs index 18568c2..bf015b3 100644 --- a/src/parsers/pyreport/mod.rs +++ b/src/parsers/pyreport/mod.rs @@ -41,8 +41,32 @@ pub fn parse_pyreport( chunks_file: &File, out_path: PathBuf, ) -> Result { - let mut report_builder = SqliteReportBuilder::new(out_path)?; + parse_pyreport_with_builder( + report_json_file, + chunks_file, + SqliteReportBuilder::new(out_path)?, + ) +} + +/// See [`parse_pyreport`] +pub fn parse_pyreport_with_seed( + report_json_file: &File, + chunks_file: &File, + out_path: PathBuf, + seed: u64, +) -> Result { + parse_pyreport_with_builder( + report_json_file, + chunks_file, + SqliteReportBuilder::new_with_seed(out_path, seed)?, + ) +} +fn parse_pyreport_with_builder( + report_json_file: &File, + chunks_file: &File, + mut report_builder: SqliteReportBuilder, +) -> Result { // Encapsulate all of this in a block so that `report_builder_tx` gets torn down // at the end. Otherwise, it'll hold onto a reference to `report_builder` // and prevent us from consuming `report_builder` to actually build a diff --git a/src/parsers/pyreport/report_json.rs b/src/parsers/pyreport/report_json.rs index b318de9..bf61f2b 100644 --- a/src/parsers/pyreport/report_json.rs +++ b/src/parsers/pyreport/report_json.rs @@ -17,36 +17,6 @@ use crate::report::{models, Report, ReportBuilder}; pub type ReportOutputStream = Stateful>; -/// Build a hopefully-unique name for a context from various fields in a session -/// object in a report JSON. -/// -/// Names look like: -/// ```notrust -/// [{timestamp}] {ci_job_name}: {ci_run_url} ({original_session_id}) -/// ``` -fn build_context_name_from_session(session_id: usize, session: &JsonVal) -> PResult { - let JsonVal::Object(values) = session else { - return Err(ErrMode::Cut(ContextError::new())); - }; - Ok(format!( - "[{}] {}: {} ({})", - values - .get("d") - .and_then(JsonVal::as_f64) - .map(|f| i64::to_string(&(f as i64))) - .unwrap_or("unknown".to_string()), - values - .get("j") - .and_then(JsonVal::as_str) - .unwrap_or("unknown"), - values - .get("u") - .and_then(JsonVal::as_str) - .unwrap_or("unknown"), - session_id, - )) -} - /// Parses a key-value pair where the key is a filename and the value is a /// `ReportFileSummary`. We primarily care about the chunks_index field and can /// compute the totals on-demand later. @@ -209,68 +179,36 @@ pub fn report_session>( let Ok(session_index) = session_index.parse::() else { return Err(ErrMode::Cut(ContextError::new())); }; + let JsonVal::Object(values) = encoded_session else { + return Err(ErrMode::Cut(ContextError::new())); + }; - let context_name = build_context_name_from_session(session_index, &encoded_session)?; - - let context = buf - .state - .report_builder - .insert_context(models::ContextType::Upload, context_name.as_str()) - .map_err(|e| ErrMode::from_external_error(buf, ErrorKind::Fail, e))?; - - let upload_details = models::UploadDetails { - context_id: context.id, - timestamp: encoded_session - .get("d") - .and_then(JsonVal::as_f64) - .map(|f| f as i64), - raw_upload_url: encoded_session - .get("a") - .and_then(JsonVal::as_str) - .map(str::to_owned), - flags: encoded_session.get("f").cloned(), - provider: encoded_session - .get("c") - .and_then(JsonVal::as_str) - .map(str::to_owned), - build: encoded_session - .get("n") - .and_then(JsonVal::as_str) - .map(str::to_owned), - name: encoded_session - .get("N") - .and_then(JsonVal::as_str) - .map(str::to_owned), - job_name: encoded_session - .get("j") - .and_then(JsonVal::as_str) - .map(str::to_owned), - ci_run_url: encoded_session - .get("u") - .and_then(JsonVal::as_str) - .map(str::to_owned), - state: encoded_session - .get("p") - .and_then(JsonVal::as_str) - .map(str::to_owned), - env: encoded_session - .get("e") - .and_then(JsonVal::as_str) - .map(str::to_owned), - session_type: encoded_session + let raw_upload = models::RawUpload { + timestamp: values.get("d").and_then(JsonVal::as_f64).map(|f| f as i64), + raw_upload_url: values.get("a").and_then(JsonVal::as_str).map(str::to_owned), + flags: values.get("f").cloned(), + provider: values.get("c").and_then(JsonVal::as_str).map(str::to_owned), + build: values.get("n").and_then(JsonVal::as_str).map(str::to_owned), + name: values.get("N").and_then(JsonVal::as_str).map(str::to_owned), + job_name: values.get("j").and_then(JsonVal::as_str).map(str::to_owned), + ci_run_url: values.get("u").and_then(JsonVal::as_str).map(str::to_owned), + state: values.get("p").and_then(JsonVal::as_str).map(str::to_owned), + env: values.get("e").and_then(JsonVal::as_str).map(str::to_owned), + session_type: values .get("st") .and_then(JsonVal::as_str) .map(str::to_owned), - session_extras: encoded_session.get("se").cloned(), + session_extras: values.get("se").cloned(), + ..Default::default() }; - let _ = buf + let raw_upload = buf .state .report_builder - .insert_upload_details(upload_details) + .insert_raw_upload(raw_upload) .map_err(|e| ErrMode::from_external_error(buf, ErrorKind::Fail, e))?; - Ok((session_index, context.id)) + Ok((session_index, raw_upload.id)) } /// Parses the JSON object that corresponds to the "files" key. Because there @@ -363,61 +301,6 @@ mod tests { use super::*; use crate::parsers::json::JsonMap; - #[test] - fn test_build_context_name_from_session() { - assert_eq!( - build_context_name_from_session(0, &json!({})), - Ok("[unknown] unknown: unknown (0)".to_string()) - ); - assert_eq!( - build_context_name_from_session(3, &json!({"d": 1234})), - Ok("[1234] unknown: unknown (3)".to_string()) - ); - assert_eq!( - build_context_name_from_session(5, &json!({"j": "codecov-rs CI"})), - Ok("[unknown] codecov-rs CI: unknown (5)".to_string()) - ); - assert_eq!( - build_context_name_from_session(0, &json!({"u": "https://example.com"})), - Ok("[unknown] unknown: https://example.com (0)".to_string()) - ); - assert_eq!( - build_context_name_from_session(0, &json!({"d": 1234, "j": "codecov-rs CI"})), - Ok("[1234] codecov-rs CI: unknown (0)".to_string()) - ); - assert_eq!( - build_context_name_from_session(0, &json!({"d": 1234, "u": "https://example.com"})), - Ok("[1234] unknown: https://example.com (0)".to_string()) - ); - assert_eq!( - build_context_name_from_session( - 0, - &json!({"j": "codecov-rs CI", "u": "https://example.com"}) - ), - Ok("[unknown] codecov-rs CI: https://example.com (0)".to_string()) - ); - assert_eq!( - build_context_name_from_session( - 0, - &json!({"d": 1234, "j": "codecov-rs CI", "u": "https://example.com"}) - ), - Ok("[1234] codecov-rs CI: https://example.com (0)".to_string()) - ); - assert_eq!( - build_context_name_from_session( - 0, - &json!({"d": 1234, "j": "codecov-rs CI", "u": "https://example.com", "n": null, "a": "https://foo.bar", "st": "carriedforward", "se": {}}) - ), - Ok("[1234] codecov-rs CI: https://example.com (0)".to_string()) - ); - - // Malformed - assert_eq!( - build_context_name_from_session(0, &json!([]),), - Err(ErrMode::Cut(ContextError::new())), - ); - } - fn test_report_file(path: &str, input: &str) -> PResult<(usize, i64)> { let ctx = setup(); let mut buf = TestStream { @@ -490,42 +373,32 @@ mod tests { ); } - // This helper is for sessions that include "j" but not "d" or "u". - // Name-building behavior is tested separately + covered in the - // `fully_populated` test case. - fn test_report_session(job_name: Option<&str>, input: &str) -> PResult<(usize, i64)> { + fn test_report_session( + job_name: Option<&str>, + upload_id: i64, + input: &str, + ) -> PResult<(usize, i64)> { let ctx = setup(); let mut buf = TestStream { input, state: ctx.parse_ctx, }; - let job_name_str = job_name.unwrap_or("unknown"); - let expected_name = format!("[unknown] {job_name_str}: unknown (0)"); - - let inserted_model = models::Context { - id: hash_id(expected_name.as_str()), - context_type: models::ContextType::Upload, - name: expected_name.clone(), - }; - - let inserted_details = models::UploadDetails { - context_id: inserted_model.id, + let inserted_upload = models::RawUpload { job_name: job_name.map(str::to_owned), ..Default::default() }; buf.state .report_builder - .expect_insert_context() - .with(eq(models::ContextType::Upload), eq(expected_name)) - .return_once(move |_, _| Ok(inserted_model)); - - buf.state - .report_builder - .expect_insert_upload_details() - .with(eq(inserted_details)) - .return_once(|details| Ok(details)); + .expect_insert_raw_upload() + .with(eq(inserted_upload)) + .return_once(move |upload| { + Ok(models::RawUpload { + id: upload_id, + ..upload + }) + }); report_session.parse_next(&mut buf) } @@ -533,15 +406,18 @@ mod tests { #[test] fn test_report_session_simple_valid_case() { assert_eq!( - test_report_session(Some("codecov-rs CI"), "\"0\": {\"j\": \"codecov-rs CI\"}",), - Ok((0, hash_id("[unknown] codecov-rs CI: unknown (0)"))) + test_report_session( + Some("codecov-rs CI"), + 5, + "\"0\": {\"j\": \"codecov-rs CI\"}", + ), + Ok((0, 5)) ); } #[test] fn test_report_session_fully_populated() { let ctx = setup(); - let session_id = 0; let timestamp = 1704827412; let job_name = "codecov-rs CI"; let ci_run_url = "https://github.com/codecov/codecov-rs/actions/runs/7465738121"; @@ -565,15 +441,7 @@ mod tests { state: ctx.parse_ctx, }; - let context_name = format!("[{timestamp}] {job_name}: {ci_run_url} ({session_id})"); - let inserted_model = models::Context { - id: hash_id(context_name.as_str()), - context_type: models::ContextType::Upload, - name: context_name.clone(), - }; - - let inserted_details = models::UploadDetails { - context_id: inserted_model.id, + let inserted_upload = models::RawUpload { timestamp: Some(timestamp), raw_upload_url: Some( "v4/raw/2024-01-09////340c0c0b-a955-46a0-9de9-3a9b5f2e81e2.txt" @@ -589,30 +457,22 @@ mod tests { env: Some("env".to_string()), session_type: Some("uploaded".to_string()), session_extras: Some(JsonVal::Object(JsonMap::new())), + ..Default::default() }; buf.state .report_builder - .expect_insert_context() - .with(eq(models::ContextType::Upload), eq(context_name.clone())) - .return_once(move |_, _| Ok(inserted_model)); - - buf.state - .report_builder - .expect_insert_upload_details() - .with(eq(inserted_details)) - .return_once(|details| Ok(details)); + .expect_insert_raw_upload() + .with(eq(inserted_upload)) + .return_once(|upload| Ok(models::RawUpload { id: 1337, ..upload })); - assert_eq!( - report_session.parse_next(&mut buf), - Ok((0, hash_id(context_name.as_str()))) - ); + assert_eq!(report_session.parse_next(&mut buf), Ok((0, 1337))); } #[test] fn test_report_session_malformed_session_index() { assert_eq!( - test_report_session(Some("codecov-rs CI"), "'0\": {\"j\": \"codecov-rs CI\"}",), + test_report_session(Some("codecov-rs CI"), 5, "'0\": {\"j\": \"codecov-rs CI\"}",), Err(ErrMode::Backtrack(ContextError::new())) ); } @@ -620,7 +480,11 @@ mod tests { #[test] fn test_report_session_session_index_not_numeric() { assert_eq!( - test_report_session(Some("codecov-rs CI"), "\"str\": {\"j\": \"codecov-rs CI\"}",), + test_report_session( + Some("codecov-rs CI"), + 5, + "\"str\": {\"j\": \"codecov-rs CI\"}", + ), Err(ErrMode::Cut(ContextError::new())) ); } @@ -630,6 +494,7 @@ mod tests { assert_eq!( test_report_session( Some("codecov-rs CI"), + 5, "\"3.34\": {\"j\": \"codecov-rs CI\"}", ), Err(ErrMode::Cut(ContextError::new())) @@ -639,23 +504,23 @@ mod tests { #[test] fn test_report_session_missing_job_key() { assert_eq!( - test_report_session(None, "\"0\": {\"x\": \"codecov-rs CI\"}",), - Ok((0, hash_id("[unknown] unknown: unknown (0)"))) + test_report_session(None, 5, "\"0\": {\"x\": \"codecov-rs CI\"}",), + Ok((0, 5)) ); } #[test] fn test_report_session_job_key_wrong_type() { assert_eq!( - test_report_session(None, "\"0\": {\"j\": []}",), - Ok((0, hash_id("[unknown] unknown: unknown (0)"))) + test_report_session(None, 5, "\"0\": {\"j\": []}",), + Ok((0, 5)) ); } #[test] fn test_report_session_encoded_session_wrong_type() { assert_eq!( - test_report_session(Some("codecov-rs CI"), "\"0\": [\"j\", []]",), + test_report_session(Some("codecov-rs CI"), 5, "\"0\": [\"j\", []]",), Err(ErrMode::Cut(ContextError::new())) ); } @@ -755,7 +620,7 @@ mod tests { // Name-building behavior is tested separately + covered in the // `fully_populated` test case. fn test_report_sessions_dict( - job_names: &[(usize, Option<&str>)], + jobs_with_raw_upload_ids: &[(Option<&str>, i64)], input: &str, ) -> PResult> { let ctx = setup(); @@ -764,32 +629,22 @@ mod tests { state: ctx.parse_ctx, }; - for (i, job_name) in job_names.iter() { - let job_name_str = job_name.unwrap_or("unknown"); - let expected_name = format!("[unknown] {job_name_str}: unknown ({i})"); - let inserted_context = models::Context { - id: hash_id(expected_name.as_str()), - context_type: models::ContextType::Upload, - name: expected_name.clone(), - }; - - let inserted_details = models::UploadDetails { - context_id: inserted_context.id, + for (job_name, upload_id) in jobs_with_raw_upload_ids.iter().cloned() { + let inserted_upload = models::RawUpload { job_name: job_name.map(str::to_owned), ..Default::default() }; buf.state .report_builder - .expect_insert_context() - .with(eq(models::ContextType::Upload), eq(expected_name)) - .return_once(move |_, _| Ok(inserted_context)); - - buf.state - .report_builder - .expect_insert_upload_details() - .with(eq(inserted_details)) - .return_once(|details| Ok(details)); + .expect_insert_raw_upload() + .with(eq(inserted_upload)) + .return_once(move |upload| { + Ok(models::RawUpload { + id: upload_id, + ..upload + }) + }); } report_sessions_dict.parse_next(&mut buf) @@ -799,10 +654,10 @@ mod tests { fn test_report_sessions_dict_single_valid_session() { assert_eq!( test_report_sessions_dict( - &[(0, Some("codecov-rs CI"))], + &[(Some("codecov-rs CI"), 5)], "{\"0\": {\"j\": \"codecov-rs CI\"}}", ), - Ok(HashMap::from([expected_context_tuple(0, "codecov-rs CI")])) + Ok(HashMap::from([(0, 5)])) ); } @@ -810,13 +665,10 @@ mod tests { fn test_report_sessions_dict_multiple_valid_sessions() { assert_eq!( test_report_sessions_dict( - &[(0, Some("codecov-rs CI")), (1, Some("codecov-rs CI 2"))], + &[(Some("codecov-rs CI"), 5), (Some("codecov-rs CI 2"), 10)], "{\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"}}", ), - Ok(HashMap::from([ - expected_context_tuple(0, "codecov-rs CI"), - expected_context_tuple(1, "codecov-rs CI 2") - ])) + Ok(HashMap::from([(0, 5), (1, 10),])) ); } @@ -824,7 +676,7 @@ mod tests { fn test_report_sessions_dict_multiple_valid_sessions_trailing_comma() { assert_eq!( test_report_sessions_dict( - &[(0, Some("codecov-rs CI")), (1, Some("codecov-rs CI 2"))], + &[(Some("codecov-rs CI"), 5), (Some("codecov-rs CI 2"), 10)], "{\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"},}", ), Err(ErrMode::Cut(ContextError::new())) @@ -837,21 +689,18 @@ mod tests { // the behavior that we want. we want to error assert_eq!( test_report_sessions_dict( - &[(0, Some("codecov-rs CI")), (0, Some("codecov-rs CI 2"))], + &[(Some("codecov-rs CI"), 5), (Some("codecov-rs CI 2"), 10)], "{\"0\": {\"j\": \"codecov-rs CI\"}, \"0\": {\"j\": \"codecov-rs CI 2\"}}", ), - Ok(HashMap::from([expected_context_tuple( - 0, - "codecov-rs CI 2" - )])) + Ok(HashMap::from([(0, 10)])) ); } #[test] fn test_report_sessions_dict_single_malformed_session() { assert_eq!( - test_report_sessions_dict(&[(0, None)], "{\"0\": {\"xj\": \"codecov-rs CI\"}}",), - Ok(HashMap::from([expected_context_tuple(0, "unknown")])) + test_report_sessions_dict(&[(None, 5)], "{\"0\": {\"xj\": \"codecov-rs CI\"}}",), + Ok(HashMap::from([(0, 5)])) ); } @@ -859,13 +708,10 @@ mod tests { fn test_report_sessions_dict_invalid_session_after_valid_session() { assert_eq!( test_report_sessions_dict( - &[(0, Some("codecov-rs CI")), (1, None)], + &[(Some("codecov-rs CI"), 5), (None, 10)], "{\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"xj\": \"codecov-rs CI 2\"}}", ), - Ok(HashMap::from([ - expected_context_tuple(0, "codecov-rs CI"), - expected_context_tuple(1, "unknown") - ])) + Ok(HashMap::from([(0, 5), (1, 10),])) ); } @@ -873,7 +719,7 @@ mod tests { fn test_report_sessions_dict_wrong_type() { assert_eq!( test_report_sessions_dict( - &[(0, Some("codecov-rs CI"))], + &[(Some("codecov-rs CI"), 5)], "{\"0\": [\"j\": \"codecov-rs CI\"}]", ), Err(ErrMode::Cut(ContextError::new())) @@ -887,7 +733,7 @@ mod tests { fn test_report_json( paths: &[&str], - job_names: &[&str], + jobs_with_raw_upload_ids: &[(&str, i64)], input: &str, ) -> PResult<(HashMap, HashMap)> { let ctx = setup(); @@ -908,68 +754,52 @@ mod tests { .return_once(move |_| Ok(inserted_file)); } - for (i, job_name) in job_names.iter().enumerate() { - let expected_name = format!("[unknown] {job_name}: unknown ({i})"); - let inserted_context = models::Context { - id: hash_id(expected_name.as_str()), - context_type: models::ContextType::Upload, - name: expected_name.clone(), - }; - - let inserted_details = models::UploadDetails { - context_id: inserted_context.id, + for (job_name, raw_upload_id) in jobs_with_raw_upload_ids.iter().cloned() { + let inserted_upload = models::RawUpload { job_name: Some(job_name.to_string()), ..Default::default() }; buf.state .report_builder - .expect_insert_context() - .with(eq(models::ContextType::Upload), eq(expected_name)) - .return_once(move |_, _| Ok(inserted_context)); - - buf.state - .report_builder - .expect_insert_upload_details() - .with(eq(inserted_details)) - .return_once(|details| Ok(details)); + .expect_insert_raw_upload() + .with(eq(inserted_upload)) + .return_once(move |upload| { + Ok(models::RawUpload { + id: raw_upload_id, + ..upload + }) + }); } parse_report_json.parse_next(&mut buf) } - fn expected_context_tuple(session_id: usize, job_name: &str) -> (usize, i64) { - ( - session_id, - hash_id(format!("[unknown] {job_name}: unknown ({session_id})").as_str()), - ) - } - #[test] fn test_report_json_simple_valid_case() { assert_eq!(test_report_json( &["src/report.rs"], - &["codecov-rs CI"], + &[("codecov-rs CI", 5)], "{\"files\": {\"src/report.rs\": [0, {}, [], null]}, \"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}}}", - ), Ok((HashMap::from([(0, hash_id("src/report.rs"))]), HashMap::from([expected_context_tuple(0, "codecov-rs CI")])))) + ), Ok((HashMap::from([(0, hash_id("src/report.rs"))]), HashMap::from([(0, 5)])))) } #[test] fn test_report_json_two_files_two_sessions() { assert_eq!(test_report_json( &["src/report.rs", "src/report/models.rs"], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"files\": {\"src/report.rs\": [0, {}, [], null], \"src/report/models.rs\": [1, {}, [], null]}, \"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"}}}", - ), Ok((HashMap::from([(0, hash_id("src/report.rs")), (1, hash_id("src/report/models.rs"))]), HashMap::from([expected_context_tuple(0, "codecov-rs CI"), expected_context_tuple(1, "codecov-rs CI 2")])))); + ), Ok((HashMap::from([(0, hash_id("src/report.rs")), (1, hash_id("src/report/models.rs"))]), HashMap::from([(0, 5), (1, 10)])))); } #[test] fn test_report_json_empty_files() { assert_eq!(test_report_json( &[], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"files\": {}, \"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"}}}", - ), Ok((HashMap::new(), HashMap::from([expected_context_tuple(0, "codecov-rs CI"), expected_context_tuple(1, "codecov-rs CI 2")])))); + ), Ok((HashMap::new(), HashMap::from([(0, 5), (1, 10)])))); } #[test] @@ -993,7 +823,7 @@ mod tests { fn test_report_json_sessions_before_files() { assert_eq!(test_report_json( &["src/report.rs", "src/report/models.rs"], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"}}, \"files\": {\"src/report.rs\": [0, {}, [], null], \"src/report/models.rs\": [1, {}, [], null]}}", ), Err(ErrMode::Cut(ContextError::new()))); } @@ -1002,7 +832,7 @@ mod tests { fn test_report_json_missing_files() { assert_eq!(test_report_json( &["src/report.rs", "src/report/models.rs"], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"}}}", ), Err(ErrMode::Cut(ContextError::new()))); } @@ -1011,7 +841,7 @@ mod tests { fn test_report_json_missing_sessions() { assert_eq!(test_report_json( &["src/report.rs", "src/report/models.rs"], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"files\": {\"src/report.rs\": [0, {}, [], null], \"src/report/models.rs\": [1, {}, [], null]}}", ), Err(ErrMode::Cut(ContextError::new()))); } @@ -1020,7 +850,7 @@ mod tests { fn test_report_json_one_invalid_file() { assert_eq!(test_report_json( &["src/report.rs", "src/report/models.rs"], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"files\": {\"src/report.rs\": [0, {}, [], null], \"src/report/models.rs\": [null, {}, [], null]}, \"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}, \"1\": {\"j\": \"codecov-rs CI 2\"}}}", ), Err(ErrMode::Cut(ContextError::new()))); } @@ -1029,7 +859,7 @@ mod tests { fn test_report_json_one_invalid_session() { assert_eq!(test_report_json( &["src/report.rs", "src/report/models.rs"], - &["codecov-rs CI", "codecov-rs CI 2"], + &[("codecov-rs CI", 5), ("codecov-rs CI 2", 10)], "{\"files\": {\"src/report.rs\": [0, {}, [], null], \"src/report/models.rs\": [1, {}, [], null]}, \"sessions\": {\"0\": {\"j\": \"codecov-rs CI\"}, \"j\": {\"xj\": \"codecov-rs CI 2\"}}}", ), Err(ErrMode::Cut(ContextError::new()))); } diff --git a/src/parsers/pyreport/utils.rs b/src/parsers/pyreport/utils.rs index e9fc690..23fe23e 100644 --- a/src/parsers/pyreport/utils.rs +++ b/src/parsers/pyreport/utils.rs @@ -64,6 +64,7 @@ fn save_line_session>( ctx: &mut ParseCtx, ) -> Result { let file_id = ctx.report_json_files[&ctx.chunk.index]; + let session_id = ctx.report_json_sessions[&line_session.session_id]; // The chunks file crams three of our model fields into the same "coverage" // field. We have to separate them. @@ -74,6 +75,7 @@ fn save_line_session>( .db .report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: session_id, source_file_id: file_id, line_no: ctx.chunk.current_line, coverage_type: *coverage_type, @@ -83,19 +85,6 @@ fn save_line_session>( ..Default::default() })?; - // We already created a `Context` for each session when processing the report - // JSON. Get the `Context` ID for the current session and then associate it with - // our new `CoverageSample`. - let session_id = ctx.report_json_sessions[&line_session.session_id]; - let _ = ctx - .db - .report_builder - .associate_context(models::ContextAssoc { - context_id: session_id, - sample_id: Some(coverage_sample.id), - ..Default::default() - })?; - // Check for and insert any additional branches data that we have. if let Some(Some(missing_branches)) = &line_session.branches { for branch in missing_branches { @@ -104,8 +93,9 @@ fn save_line_session>( .db .report_builder .insert_branches_data(models::BranchesData { + raw_upload_id: session_id, source_file_id: file_id, - sample_id: coverage_sample.id, + local_sample_id: coverage_sample.local_sample_id, hits: 0, // Chunks file only records missing branches branch_format, branch: branch_serialized, @@ -121,8 +111,9 @@ fn save_line_session>( .db .report_builder .insert_method_data(models::MethodData { + raw_upload_id: session_id, source_file_id: file_id, - sample_id: Some(coverage_sample.id), + local_sample_id: coverage_sample.local_sample_id, line_no: Some(ctx.chunk.current_line), hit_complexity_paths: covered, total_complexity: total, @@ -143,8 +134,9 @@ fn save_line_session>( _ => 0, }; ctx.db.report_builder.insert_span_data(models::SpanData { + raw_upload_id: session_id, source_file_id: file_id, - sample_id: Some(coverage_sample.id), + local_sample_id: Some(coverage_sample.local_sample_id), hits, start_line: Some(ctx.chunk.current_line), start_col: start_col.map(|x| x as i64), @@ -180,7 +172,8 @@ pub fn save_report_line>( .report_builder .associate_context(models::ContextAssoc { context_id, - sample_id: Some(coverage_sample.id), + raw_upload_id: coverage_sample.raw_upload_id as i64, + local_sample_id: Some(coverage_sample.local_sample_id), ..Default::default() })?; } @@ -279,49 +272,40 @@ mod tests { parse_ctx: &mut ParseCtx>, sequence: &mut mockall::Sequence, ) -> models::CoverageSample { - let session_context_id = parse_ctx.report_json_sessions[&line_session.session_id]; + let raw_upload_id = parse_ctx.report_json_sessions[&line_session.session_id]; let source_file_id = parse_ctx.report_json_files[&parse_ctx.chunk.index]; let (hits, hit_branches, total_branches) = separate_pyreport_coverage(&line_session.coverage); + let line_no = parse_ctx.chunk.current_line; + let local_sample_id = rand::random(); let inserted_coverage_sample = models::CoverageSample { - id: uuid::Uuid::new_v4(), + raw_upload_id, + local_sample_id, source_file_id, - line_no: parse_ctx.chunk.current_line, + line_no, coverage_type, hits, hit_branches, total_branches, + ..Default::default() }; parse_ctx .db .report_builder .expect_insert_coverage_sample() .with(eq(models::CoverageSample { - id: uuid::Uuid::nil(), + local_sample_id: 0, ..inserted_coverage_sample })) .return_once(move |mut sample| { - sample.id = inserted_coverage_sample.id; + sample.local_sample_id = local_sample_id; Ok(sample) }) .times(1) .in_sequence(sequence); - parse_ctx - .db - .report_builder - .expect_associate_context() - .with(eq(models::ContextAssoc { - context_id: session_context_id, - sample_id: Some(inserted_coverage_sample.id), - ..Default::default() - })) - .returning(|assoc| Ok(assoc)) - .times(1) - .in_sequence(sequence); - if let Some(Some(missing_branches)) = &line_session.branches { for branch in missing_branches { let (branch_format, branch_serialized) = format_pyreport_branch(branch); @@ -330,15 +314,16 @@ mod tests { .report_builder .expect_insert_branches_data() .with(eq(models::BranchesData { + raw_upload_id, source_file_id, - sample_id: inserted_coverage_sample.id, + local_sample_id, hits: 0, branch_format, branch: branch_serialized, ..Default::default() })) .return_once(move |mut branch| { - branch.id = uuid::Uuid::new_v4(); + branch.local_branch_id = rand::random(); Ok(branch) }) .times(1) @@ -359,15 +344,16 @@ mod tests { .report_builder .expect_insert_method_data() .with(eq(models::MethodData { + raw_upload_id, source_file_id, - sample_id: Some(inserted_coverage_sample.id), - line_no: Some(inserted_coverage_sample.line_no), + local_sample_id, + line_no: Some(line_no), hit_complexity_paths: covered, total_complexity: total, ..Default::default() })) .return_once(move |mut method| { - method.id = uuid::Uuid::new_v4(); + method.local_method_id = rand::random(); Ok(method) }) .times(1) @@ -396,17 +382,18 @@ mod tests { .report_builder .expect_insert_span_data() .with(eq(models::SpanData { + raw_upload_id, source_file_id, - sample_id: Some(inserted_coverage_sample.id), + local_sample_id: Some(local_sample_id), hits, - start_line: Some(inserted_coverage_sample.line_no), + start_line: Some(line_no), start_col: start_col.map(|x| x as i64), - end_line: Some(inserted_coverage_sample.line_no), + end_line: Some(line_no), end_col: end_col.map(|x| x as i64), ..Default::default() })) .return_once(move |mut span| { - span.id = uuid::Uuid::new_v4(); + span.local_span_id = rand::random(); Ok(span) }) .times(1) @@ -722,7 +709,8 @@ mod tests { .expect_associate_context() .with(eq(models::ContextAssoc { context_id: 50, - sample_id: Some(inserted_sample.id), + raw_upload_id: inserted_sample.raw_upload_id, + local_sample_id: Some(inserted_sample.local_sample_id), ..Default::default() })) .returning(|assoc| Ok(assoc)) @@ -733,7 +721,8 @@ mod tests { .expect_associate_context() .with(eq(models::ContextAssoc { context_id: 51, - sample_id: Some(inserted_sample.id), + raw_upload_id: inserted_sample.raw_upload_id, + local_sample_id: Some(inserted_sample.local_sample_id), ..Default::default() })) .returning(|assoc| Ok(assoc)) diff --git a/src/report/mod.rs b/src/report/mod.rs index b30c7fe..a38abd6 100644 --- a/src/report/mod.rs +++ b/src/report/mod.rs @@ -25,7 +25,7 @@ pub trait Report { &self, file: &models::SourceFile, ) -> Result>; - fn get_details_for_upload(&self, upload: &models::Context) -> Result; + fn list_raw_uploads(&self) -> Result>; /// Merges another report into this one. Does not modify the other report. fn merge(&mut self, other: &Self) -> Result<()>; @@ -48,25 +48,30 @@ pub trait ReportBuilder { ) -> Result; /// Create a [`models::CoverageSample`] record and return it. The passed-in - /// model's `id` field is ignored and overwritten with a new UUIDv4. + /// model's `local_sample_id` field is ignored and overwritten with a value + /// that is unique among all `CoverageSample`s with the same + /// `raw_upload_id`. fn insert_coverage_sample( &mut self, sample: models::CoverageSample, ) -> Result; /// Create a [`models::BranchesData`] record and return it. The passed-in - /// model's `id` field is ignored and overwritten with a new UUIDv4. + /// model's `local_branch_id` field is ignored and overwritten with a value + /// that is unique among all `BranchesData`s with the same `raw_upload_id`. fn insert_branches_data( &mut self, branch: models::BranchesData, ) -> Result; /// Create a [`models::MethodData`] record and return it. The passed-in - /// model's `id` field is ignored and overwritten with a new UUIDv4. + /// model's `local_method_id` field is ignored and overwritten with a value + /// that is unique among all `MethodData`s with the same `raw_upload_id`. fn insert_method_data(&mut self, method: models::MethodData) -> Result; /// Create a [`models::SpanData`] record and return it. The passed-in - /// model's `id` field is ignored and overwritten with a new UUIDv4. + /// model's `local_span_id` field is ignored and overwritten with a value + /// that is unique among all `SpanData`s with the same `raw_upload_id`. fn insert_span_data(&mut self, span: models::SpanData) -> Result; /// Create a [`models::ContextAssoc`] record that associates a @@ -77,11 +82,9 @@ pub trait ReportBuilder { assoc: models::ContextAssoc, ) -> Result; - /// Create a [`models::UploadDetails`] record and return it. - fn insert_upload_details( - &mut self, - upload_details: models::UploadDetails, - ) -> Result; + /// Create a [`models::RawUpload`] record and return it. + fn insert_raw_upload(&mut self, upload_details: models::RawUpload) + -> Result; /// Consume `self` and return a [`Report`]. fn build(self) -> Result; diff --git a/src/report/models.rs b/src/report/models.rs index 40bcefb..dffbe8a 100644 --- a/src/report/models.rs +++ b/src/report/models.rs @@ -2,44 +2,80 @@ * Models for coverage data to be used by [`crate::report::ReportBuilder`] * and [`crate::report::Report`] implementations. * - * An overview of the models and their relationships: - * - Each source file in a report should have a [`SourceFile`] record. - * - Each line in that source file, whether it's a statement, a branch, or a - * method declaration, should have a [`CoverageSample`] record with its - * `source_file_id` field pointed at its source file. - * - For lines/statements/method declarations, the `hits` field should - * contain a hit count - * - For branches, the `hit_branches` and `total_branches` fields should - * be filled in instead - * - The [`Context`] and [`ContextAssoc`] models can be used to tag - * measurements with context (e.g. "these measurements were on Windows", - * "these measurements were from TestSuiteFoo") and enable - * querying/filtering. - * - [`BranchesData`], [`MethodData`], and [`SpanData`] are optional. These - * models are for information that is not provided in every format or is - * provided in different shapes, and records should be tied to a - * [`CoverageSample`]. - * - [`BranchesData`] can store information about which specific branches - * were hit or missed - * - [`MethodData`] can store information including cyclomatic complexity - * or aggregated line/branch coverage - * - [`SpanData`] can store coverage information that isn't presented - * line-by-line. Some formats report that a range from line X to line Y - * is covered, or that only part of a single line is covered. - * - [`ReportTotals`] and [`CoverageTotals`] aggregate coverage data for a - * report/subset into useful metrics. + * ## Data model overview * - * Some choices were made that make merging multiple reports together very + * ### [`RawUpload`] + * Each Codecov upload should have a `RawUpload` record with a random ID. + * + * ### [`SourceFile`] + * Each source file we have coverage data for should have a `SourceFile` + * record. + * + * ### [`CoverageSample`] + * An individual coverage measurement for a line of code. If the line is a + * statement or method declaration, the `hits` field records the number of + * times the line was hit. If the line is the root of a set of branches, the + * `hit_branches` and `total_branches` fields will be filled in instead. + * + * In the simple case, there will be a `CoverageSample` for each line, in + * each source file, in each Codecov upload. If there are multiple Codecov + * uploads, there will be multiple `CoverageSample` records for `foo.rs:32`. + * + * ### [`BranchesData`] + * A `CoverageSample` record that describes a branch may have associated + * `BranchesData` records with the coverage status of each specific branch. + * If `foo.rs:32` is an if statement, its `CoverageSample` will record + * whether 0/2, 1/2, or 2/2 branches were covered, and `BranchesData` will + * have a record for whether the "true" branch is covered and another for + * the "false" branch. + * + * Specific branch information is not always available. In Codecov's + * pyreport format, only specific missed branches are recorded. + * + * ### [`MethodData`] + * A `CoverageSample` record that describes a method declaration may have a + * `MethodData` record with extra method-specific data like cyclomatic + * complexity. + * + * ### [`SpanData`] + * A `SpanData` record represents a coverage measurement that isn't scoped + * to a line of code. Some formats take measurements that cover ranges of + * lines, and some take measurements that can cover a subset of a single + * line. + * + * (TODO) The relationship between a `SpanData` and a `CoverageSample` isn't + * settled. + * - Synthesize a `CoverageSample` record for each line covered by a span + * measurement. Faithfully record each raw span measurement as a + * `SpanData` record, and associate it with the relevant `CoverageSample` + * records somehow. + * - Synthesize a `CoverageSample` record for each line covered by a span + * measurement. If the first or last line of a span describes a subset of + * a single line, create a `SpanData` record for that single-line subset. + * + * ### [`Context`] and [`ContextAssoc`] + * `Context` has a many-to-many relationship with `CoverageSample` and can + * link, for example, an individual test case with all the lines it covered. + * + * ### [`ReportTotals`] and [`CoverageTotals`] + * (Not actually database models) + * Aggregated coverage metrics. + * + * ## Implementation notes + * + * Some choices were made to make distributed processing / merging very * simple: - * - [`SourceFile`] and [`Context`] use hashes of their names as an ID so - * every report they appear in will come up with the same ID + * - [`SourceFile`] and [`Context`] use hashes of their names as an ID. + * Different hosts will all come up with the same ID for each. * - Measurement models ([`CoverageSample`], [`BranchesData`], - * [`MethodData`], [`SpanData`]) use UUIDv4s for IDs to essentially - * guarantee different reports will not use the same IDs + * [`MethodData`], [`SpanData`]) have a composite primary key on + * `raw_upload_id` (random per upload) and `local_*_id` (auto-increment + * int). As long as we use a unique `raw_upload_id` when processing each + * upload, it doesn't matter if `local_*_id` values are repeated. * * These properties make merging essentially just concatenation. - * [`SourceFile`]s and [`Context`] can be merged into an existing report - * with `INSERT OR IGNORE`, and the rest can be merged with a + * [`SourceFile`]s and [`Context`]s can be merged into an existing report + * with `INSERT OR IGNORE` and the rest can be merged with a * regular `INSERT` without needing to update any foreign keys or anything. * * SeaHash was chosen for hashed IDs due to: @@ -57,7 +93,6 @@ */ use strum_macros::{Display, EnumString}; -use uuid::Uuid; use crate::parsers::json::JsonVal; @@ -92,13 +127,8 @@ pub enum ContextType { /// A [`Context`] with this type represents a test case, and every /// [`CoverageSample`] associated with it is a measurement that applies /// to that test case. - TestCase, - - /// A [`Context`] with this type represents a distinct upload with coverage - /// data. For instance, a Windows test runner and a macOS test runner - /// may send coverage data for the same code in two separate uploads. #[default] - Upload, + TestCase, } /// Each source file represented in the coverage data should have a @@ -112,10 +142,16 @@ pub struct SourceFile { pub path: String, } -/// Each line in a source file should have a [`CoverageSample`] record when -/// possible, whether it's a line/statement, a branch, or a method declaration. -/// The `coverage_sample` table should be sufficient to paint green/yellow/red -/// lines in a UI. +/// A `CoverageSample` record is a single coverage measurement. There will be a +/// coverage measurement for each line of code, in each [`SourceFile`], in each +/// coverage data file, in each [`RawUpload`]. If a report contains data from +/// multiple `RawUpload`s, or if a single `RawUpload` contains multiple coverage +/// data files, then `foo.rs:32` (for example) may have multiple +/// `CoverageSample` records. +/// +/// A line should have a `CoverageSample` record whether it's a statement, a +/// method declaration, or a branch. The `coverage_sample` table should be +/// sufficient to paint green/yellow/red lines in a UI. /// /// A line is fully covered if: /// - its `coverage_type` is [`CoverageType::Line`] or [`CoverageType::Method`] @@ -134,7 +170,12 @@ pub struct SourceFile { /// value is less than its `total_branches` value (but greater than 0) #[derive(PartialEq, Debug, Clone, Default)] pub struct CoverageSample { - pub id: Uuid, + /// The ID of the [`RawUpload`] that this `CoverageSample` was created from. + pub raw_upload_id: i64, + + /// This `CoverageSample`'s ID, unique among other `CoverageSample`s from + /// the same [`RawUpload`]. + pub local_sample_id: i64, /// Should be a hash of the file's path relative to the project's root. pub source_file_id: i64, @@ -156,17 +197,24 @@ pub struct CoverageSample { pub total_branches: Option, } -/// If raw coverage data includes information about which specific branches -/// stemming from some line were or weren't covered, it can be stored here. +/// The [`CoverageSample`] record for a branch stem captures whether (for +/// example) 0/2, 1/2, or 2/2 possible branches were hit, and, if the data is +/// available, there will be an associated `BranchesData` record for each +/// possible branch recording whether that branch was hit. #[derive(PartialEq, Debug, Default, Clone)] pub struct BranchesData { - pub id: Uuid, + /// The ID of the [`RawUpload`] that this `BranchesData` was created from. + pub raw_upload_id: i64, + + /// This `BranchesData`'s ID, unique among other `BranchesData`s from + /// the same [`RawUpload`]. + pub local_branch_id: i64, /// Should be a hash of the file's path relative to the project's root. pub source_file_id: i64, /// The [`CoverageSample`] record for the line this branch stems from. - pub sample_id: Uuid, + pub local_sample_id: i64, /// The number of times this particular branch was run. pub hits: i64, @@ -180,19 +228,25 @@ pub struct BranchesData { pub branch: String, } -/// If raw coverage data includes additional metrics for methods (cyclomatic -/// complexity, aggregated branch coverage) or details like its name or -/// signature, they can be stored here. +/// The [`CoverageSample`] record for a method declaration captures how many +/// times it was hit. If there is additional method-specific data like +/// cyclomatic complexity available, we can create an associated `MethodData` +/// record to store it. #[derive(PartialEq, Debug, Default, Clone)] pub struct MethodData { - pub id: Uuid, + /// The ID of the [`RawUpload`] that this `MethodData` was created from. + pub raw_upload_id: i64, + + /// This `MethodData`'s ID, unique among other `MethodData`s from + /// the same [`RawUpload`]. + pub local_method_id: i64, /// Should be a hash of the file's path relative to the project's root. pub source_file_id: i64, /// The [`CoverageSample`] record for the line this method was declared on, /// if known. - pub sample_id: Option, + pub local_sample_id: i64, /// The line this method was declared on, in case it's known but we don't /// have a [`CoverageSample`] for it. @@ -229,7 +283,12 @@ pub struct MethodData { /// [`CoverageSample`] records for them. #[derive(PartialEq, Debug, Default, Clone)] pub struct SpanData { - pub id: Uuid, + /// The ID of the [`RawUpload`] that this `SpanData` was created from. + pub raw_upload_id: i64, + + /// This `SpanData`'s ID, unique among other `SpanData`s from + /// the same [`RawUpload`]. + pub local_span_id: i64, /// Should be a hash of the file's path relative to the project's root. pub source_file_id: i64, @@ -237,7 +296,7 @@ pub struct SpanData { /// A [`CoverageSample`] that is tied to this span, if there is a singular /// one to pick. If a span is for a subsection of a single line, we /// should be able to link a [`CoverageSample`]. - pub sample_id: Option, + pub local_sample_id: Option, /// The number of times this span was run. pub hits: i64, @@ -260,10 +319,9 @@ pub struct SpanData { pub struct ContextAssoc { /// Should be a hash of the context's `name` field. pub context_id: i64, - pub sample_id: Option, - pub branch_id: Option, - pub method_id: Option, - pub span_id: Option, + pub raw_upload_id: i64, + pub local_sample_id: Option, + pub local_span_id: Option, } /// Context that can be associated with measurements to allow querying/filtering @@ -274,18 +332,16 @@ pub struct Context { pub id: i64, pub context_type: ContextType, - /// Some sort of unique name for this context, such as a test case name or a - /// CI results URI. + /// Some sort of unique name for this context, such as a test case name. pub name: String, } -/// Details about an upload of coverage data including its flags, the path it -/// was uploaded to, the CI job that uploaded it, or a link to the results of -/// that CI job. +/// Details about a Codecov upload including its flags, the path it was uploaded +/// to, the CI job that uploaded it, and a link to the results of that CI job. #[derive(PartialEq, Debug, Default, Clone)] -pub struct UploadDetails { - /// Should be a hash of the context's `name` field. - pub context_id: i64, +pub struct RawUpload { + /// Should be a random i64. + pub id: i64, /// Unix timestamp in seconds. /// diff --git a/src/report/pyreport/chunks.rs b/src/report/pyreport/chunks.rs index 35c2227..ab3172b 100644 --- a/src/report/pyreport/chunks.rs +++ b/src/report/pyreport/chunks.rs @@ -332,25 +332,60 @@ mod tests { } fn build_sample_report(path: PathBuf) -> Result { - let mut builder = SqliteReportBuilder::new(path)?; + let mut builder = SqliteReportBuilder::new_with_seed(path, 5)?; let file_1 = builder.insert_file("src/report/report.rs".to_string())?; let file_2 = builder.insert_file("src/report/models.rs".to_string())?; + let upload_1 = builder.insert_raw_upload(models::RawUpload { + timestamp: Some(123), + raw_upload_url: Some("upload 1 url".to_string()), + flags: Some(json!(["flag on upload 1"])), + provider: Some("provider upload 1".to_string()), + build: Some("build upload 1".to_string()), + name: Some("name upload 1".to_string()), + job_name: Some("job name upload 1".to_string()), + ci_run_url: Some("ci run url upload 1".to_string()), + state: Some("state upload 1".to_string()), + env: Some("env upload 1".to_string()), + session_type: Some("type upload 1".to_string()), + session_extras: Some(json!({"k1": "v1"})), + ..Default::default() + })?; + let upload_2 = builder.insert_raw_upload(models::RawUpload { + timestamp: Some(456), + raw_upload_url: Some("upload 2 url".to_string()), + flags: Some(json!(["flag on upload 2"])), + provider: Some("provider upload 2".to_string()), + build: Some("build upload 2".to_string()), + name: Some("name upload 2".to_string()), + job_name: Some("job name upload 2".to_string()), + ci_run_url: Some("ci run url upload 2".to_string()), + state: Some("state upload 2".to_string()), + env: Some("env upload 2".to_string()), + session_type: Some("type upload 2".to_string()), + session_extras: Some(json!({"k2": "v2"})), + ..Default::default() + })?; + let line_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 1, coverage_type: models::CoverageType::Line, hits: Some(3), ..Default::default() })?; + let line_2 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 1, coverage_type: models::CoverageType::Line, hits: Some(4), ..Default::default() })?; - let line_3 = builder.insert_coverage_sample(models::CoverageSample { + let _line_3 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_2.id, line_no: 3, coverage_type: models::CoverageType::Line, @@ -359,6 +394,7 @@ mod tests { })?; let branch_sample_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 3, coverage_type: models::CoverageType::Branch, @@ -367,16 +403,18 @@ mod tests { ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_1.source_file_id, - sample_id: branch_sample_1.id, + local_sample_id: branch_sample_1.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "0:jump".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_1.source_file_id, - sample_id: branch_sample_1.id, + local_sample_id: branch_sample_1.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "1".to_string(), @@ -384,6 +422,7 @@ mod tests { })?; let branch_sample_2 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 6, coverage_type: models::CoverageType::Branch, @@ -392,32 +431,36 @@ mod tests { ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "0:jump".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "1".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 0, branch_format: models::BranchFormat::Condition, branch: "2".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 0, branch_format: models::BranchFormat::Condition, branch: "3".to_string(), @@ -425,6 +468,7 @@ mod tests { })?; let method_sample_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -432,8 +476,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_method_data(models::MethodData { + raw_upload_id: upload_1.id, source_file_id: method_sample_1.source_file_id, - sample_id: Some(method_sample_1.id), + local_sample_id: method_sample_1.local_sample_id, line_no: Some(method_sample_1.line_no), hit_branches: Some(2), total_branches: Some(4), @@ -443,6 +488,7 @@ mod tests { })?; let method_sample_2 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -450,8 +496,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_method_data(models::MethodData { + raw_upload_id: upload_1.id, source_file_id: method_sample_2.source_file_id, - sample_id: Some(method_sample_2.id), + local_sample_id: method_sample_2.local_sample_id, line_no: Some(method_sample_2.line_no), hit_branches: Some(2), total_branches: Some(4), @@ -459,6 +506,7 @@ mod tests { })?; let method_sample_3 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_2.id, line_no: 5, coverage_type: models::CoverageType::Method, @@ -466,8 +514,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_method_data(models::MethodData { + raw_upload_id: upload_2.id, source_file_id: method_sample_3.source_file_id, - sample_id: Some(method_sample_3.id), + local_sample_id: method_sample_3.local_sample_id, line_no: Some(method_sample_3.line_no), hit_complexity_paths: Some(2), total_complexity: Some(4), @@ -475,6 +524,7 @@ mod tests { })?; let line_with_partial_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 8, coverage_type: models::CoverageType::Line, @@ -482,8 +532,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_span_data(models::SpanData { + raw_upload_id: upload_1.id, source_file_id: line_with_partial_1.source_file_id, - sample_id: Some(line_with_partial_1.id), + local_sample_id: Some(line_with_partial_1.local_sample_id), start_line: Some(line_with_partial_1.line_no), start_col: Some(3), end_line: Some(line_with_partial_1.line_no), @@ -492,113 +543,37 @@ mod tests { ..Default::default() })?; - let upload_1 = builder.insert_context(models::ContextType::Upload, "codecov-rs CI")?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(line_1.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(line_2.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(branch_sample_1.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(branch_sample_2.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(method_sample_1.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(method_sample_2.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(line_with_partial_1.id), - ..Default::default() - })?; - - let upload_2 = builder.insert_context(models::ContextType::Upload, "codecov-rs CI 2")?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_2.id, - sample_id: Some(line_3.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_2.id, - sample_id: Some(method_sample_3.id), - ..Default::default() - })?; - let label_1 = builder.insert_context(models::ContextType::TestCase, "test-case")?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_1.id, - sample_id: Some(line_1.id), + raw_upload_id: upload_1.id, + local_sample_id: Some(line_1.local_sample_id), ..Default::default() })?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_1.id, - sample_id: Some(line_2.id), + raw_upload_id: upload_1.id, + local_sample_id: Some(line_2.local_sample_id), ..Default::default() })?; + let label_2 = builder.insert_context(models::ContextType::TestCase, "test-case 2")?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_2.id, - sample_id: Some(line_1.id), + raw_upload_id: upload_1.id, + local_sample_id: Some(line_1.local_sample_id), ..Default::default() })?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_2.id, - sample_id: Some(line_2.id), + raw_upload_id: upload_1.id, + local_sample_id: Some(line_2.local_sample_id), ..Default::default() })?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_2.id, - sample_id: Some(method_sample_1.id), - ..Default::default() - })?; - - let _ = builder.insert_upload_details(models::UploadDetails { - context_id: upload_1.id, - timestamp: Some(123), - raw_upload_url: Some("upload 1 url".to_string()), - flags: Some(json!(["flag on upload 1"])), - provider: Some("provider upload 1".to_string()), - build: Some("build upload 1".to_string()), - name: Some("name upload 1".to_string()), - job_name: Some("job name upload 1".to_string()), - ci_run_url: Some("ci run url upload 1".to_string()), - state: Some("state upload 1".to_string()), - env: Some("env upload 1".to_string()), - session_type: Some("type upload 1".to_string()), - session_extras: Some(json!({"k1": "v1"})), - ..Default::default() - })?; - let _ = builder.insert_upload_details(models::UploadDetails { - context_id: upload_2.id, - timestamp: Some(456), - raw_upload_url: Some("upload 2 url".to_string()), - flags: Some(json!(["flag on upload 2"])), - provider: Some("provider upload 2".to_string()), - build: Some("build upload 2".to_string()), - name: Some("name upload 2".to_string()), - job_name: Some("job name upload 2".to_string()), - ci_run_url: Some("ci run url upload 2".to_string()), - state: Some("state upload 2".to_string()), - env: Some("env upload 2".to_string()), - session_type: Some("type upload 2".to_string()), - session_extras: Some(json!({"k2": "v2"})), + raw_upload_id: upload_1.id, + local_sample_id: Some(method_sample_1.local_sample_id), ..Default::default() })?; diff --git a/src/report/pyreport/queries/chunks_file_header.sql b/src/report/pyreport/queries/chunks_file_header.sql index c966491..64a8ca5 100644 --- a/src/report/pyreport/queries/chunks_file_header.sql +++ b/src/report/pyreport/queries/chunks_file_header.sql @@ -4,8 +4,6 @@ select context.name from context -where - context.context_type <> 'Upload' ), labels_index as ( select diff --git a/src/report/pyreport/queries/files_to_report_json.sql b/src/report/pyreport/queries/files_to_report_json.sql index 38bce10..9b147c1 100644 --- a/src/report/pyreport/queries/files_to_report_json.sql +++ b/src/report/pyreport/queries/files_to_report_json.sql @@ -1,6 +1,7 @@ with samples_categorized as ( select - coverage_sample.id, + coverage_sample.raw_upload_id, + coverage_sample.local_sample_id, coverage_sample.source_file_id, coverage_sample.line_no, coverage_sample.coverage_type, @@ -16,7 +17,8 @@ from left join method_data on - method_data.sample_id = coverage_sample.id + method_data.raw_upload_id = coverage_sample.raw_upload_id + and method_data.local_sample_id = coverage_sample.local_sample_id ), source_files_with_index as ( select @@ -59,17 +61,15 @@ group by ), session_indices as ( select - cast(row_number() over (order by context.id) - 1 as text) as session_index, - context.id as context_id + cast(row_number() over (order by raw_upload.id) - 1 as text) as session_index, + raw_upload.id as raw_upload_id from - context -where - context.context_type = 'Upload' + raw_upload ), file_session_totals as ( select session_indices.session_index, - context.id, + session_indices.raw_upload_id, samples_categorized.source_file_id, count(*) as file_session_lines, sum(samples_categorized.hit) as file_session_hits, @@ -79,20 +79,10 @@ select coalesce(sum(samples_categorized.total_complexity), 0) as file_session_total_complexity from samples_categorized -left join - context_assoc -on - context_assoc.sample_id = samples_categorized.id -left join - context -on - context.id = context_assoc.context_id left join session_indices on - session_indices.context_id = context.id -where - context.context_type = 'Upload' + session_indices.raw_upload_id = samples_categorized.raw_upload_id group by 1, 2, 3 ) diff --git a/src/report/pyreport/queries/samples_to_chunks.sql b/src/report/pyreport/queries/samples_to_chunks.sql index 47fb0e2..ec472c7 100644 --- a/src/report/pyreport/queries/samples_to_chunks.sql +++ b/src/report/pyreport/queries/samples_to_chunks.sql @@ -1,24 +1,9 @@ with session_indices as ( select - row_number() over (order by context.id) - 1 as session_index, - context.id as context_id + row_number() over (order by raw_upload.id) - 1 as session_index, + raw_upload.id as raw_upload_id from - context -where - context.context_type = 'Upload' -), -upload_assocs as ( -select - context_assoc.context_id, - context_assoc.sample_id -from - context_assoc -left join - context -on - context_assoc.context_id = context.id -where - context.context_type = 'Upload' + raw_upload ), chunks_file_indices as ( select @@ -31,42 +16,18 @@ left join coverage_sample on coverage_sample.source_file_id = source_file.id -left join - upload_assocs -on - upload_assocs.sample_id = coverage_sample.id left join session_indices on - upload_assocs.context_id = session_indices.context_id + coverage_sample.raw_upload_id = session_indices.raw_upload_id group by 2 ), -other_contexts as ( -select - * -from - context -where - context.context_type <> 'Upload' -), -other_assocs as ( -select - context_assoc.context_id, - context_assoc.sample_id -from - context_assoc -left join - context -on - context_assoc.context_id = context.id -where - context.context_type <> 'Upload' -), formatted_span_data as ( select - span_data.id, - span_data.sample_id, + span_data.raw_upload_id, + span_data.local_span_id, + span_data.local_sample_id, json_array(span_data.start_col, span_data.end_col, span_data.hits) as pyreport_partial from span_data @@ -86,25 +47,24 @@ select -- The `order by` below is not strictly necessary, it just makes writing test cases easier json_group_array(branches_data.branch order by branches_data.branch) filter (where branches_data.branch is not null and branches_data.hits = 0) as missing_branches, json_group_array(json(formatted_span_data.pyreport_partial)) filter (where formatted_span_data.pyreport_partial is not null) as partials, - json_group_array(other_contexts.name) filter (where other_contexts.name is not null) as labels + json_group_array(context.name) filter (where context.name is not null) as labels from coverage_sample -left join - upload_assocs -on - upload_assocs.sample_id = coverage_sample.id left join branches_data on - branches_data.sample_id = coverage_sample.id + branches_data.raw_upload_id = coverage_sample.raw_upload_id + and branches_data.local_sample_id = coverage_sample.local_sample_id left join method_data on - method_data.sample_id = coverage_sample.id + method_data.raw_upload_id = coverage_sample.raw_upload_id + and method_data.local_sample_id = coverage_sample.local_sample_id left join formatted_span_data on - formatted_span_data.sample_id = coverage_sample.id + formatted_span_data.raw_upload_id = coverage_sample.raw_upload_id + and formatted_span_data.local_sample_id = coverage_sample.local_sample_id left join chunks_file_indices on @@ -112,17 +72,18 @@ on left join session_indices on - session_indices.context_id = upload_assocs.context_id + session_indices.raw_upload_id = coverage_sample.raw_upload_id left join - other_assocs + context_assoc on - other_assocs.sample_id = coverage_sample.id + context_assoc.raw_upload_id = coverage_sample.raw_upload_id + and context_assoc.local_sample_id = coverage_sample.local_sample_id left join - other_contexts + context on - other_contexts.id = other_assocs.context_id + context_assoc.context_id = context.id group by 1, 2, 3, 4 -order by 1, 2, 3, other_contexts.name +order by 1, 2, 3, context.name ), report_line_totals as ( select diff --git a/src/report/pyreport/queries/sessions_to_report_json.sql b/src/report/pyreport/queries/sessions_to_report_json.sql index 2ea06b4..d735fb1 100644 --- a/src/report/pyreport/queries/sessions_to_report_json.sql +++ b/src/report/pyreport/queries/sessions_to_report_json.sql @@ -1,6 +1,7 @@ with samples_categorized as ( select - coverage_sample.id, + coverage_sample.raw_upload_id, + coverage_sample.local_sample_id, coverage_sample.source_file_id, coverage_sample.line_no, coverage_sample.coverage_type, @@ -16,11 +17,12 @@ from left join method_data on - method_data.sample_id = coverage_sample.id + method_data.raw_upload_id = coverage_sample.raw_upload_id + and method_data.local_sample_id = coverage_sample.local_sample_id ) select - cast(row_number() over (order by context.id) - 1 as text) as session_index, - context.id, + cast(row_number() over (order by raw_upload.id) - 1 as text) as session_index, + raw_upload.id, count(distinct samples_categorized.source_file_id) as session_files, count(*) as session_lines, sum(samples_categorized.hit) as session_hits, @@ -30,33 +32,23 @@ select sum(iif(samples_categorized.coverage_type = 'm', 1, 0)) as session_methods, coalesce(sum(samples_categorized.hit_complexity_paths), 0) as session_hit_complexity_paths, coalesce(sum(samples_categorized.total_complexity), 0) as session_total_complexity, - upload_details.timestamp, - upload_details.raw_upload_url, - upload_details.flags, - upload_details.provider, - upload_details.build, - upload_details.name, - upload_details.job_name, - upload_details.ci_run_url, - upload_details.state, - upload_details.env, - upload_details.session_type, - upload_details.session_extras + raw_upload.timestamp, + raw_upload.raw_upload_url, + raw_upload.flags, + raw_upload.provider, + raw_upload.build, + raw_upload.name, + raw_upload.job_name, + raw_upload.ci_run_url, + raw_upload.state, + raw_upload.env, + raw_upload.session_type, + raw_upload.session_extras from samples_categorized left join - context_assoc + raw_upload on - context_assoc.sample_id = samples_categorized.id -left join - context -on - context.id = context_assoc.context_id -left join - upload_details -on - upload_details.context_id = context.id -where - context.context_type = 'Upload' + raw_upload.id = samples_categorized.raw_upload_id group by 2 diff --git a/src/report/pyreport/report_json.rs b/src/report/pyreport/report_json.rs index f67bfbd..0c5d788 100644 --- a/src/report/pyreport/report_json.rs +++ b/src/report/pyreport/report_json.rs @@ -254,7 +254,7 @@ fn sql_to_sessions_dict(report: &SqliteReport, output: &mut impl Write) -> Resul None }; - let upload_details = models::UploadDetails { + let raw_upload = models::RawUpload { timestamp: row.get(11)?, raw_upload_url: row.get::>(12)?, flags, @@ -273,18 +273,18 @@ fn sql_to_sessions_dict(report: &SqliteReport, output: &mut impl Write) -> Resul session_id, json!({ "t": totals, - "d": upload_details.timestamp, - "a": upload_details.raw_upload_url, - "f": upload_details.flags, - "c": upload_details.provider, - "n": upload_details.build, - "N": upload_details.name, - "j": upload_details.job_name, - "u": upload_details.ci_run_url, - "p": upload_details.state, - "e": upload_details.env, - "st": upload_details.session_type, - "se": upload_details.session_extras, + "d": raw_upload.timestamp, + "a": raw_upload.raw_upload_url, + "f": raw_upload.flags, + "c": raw_upload.provider, + "n": raw_upload.build, + "N": raw_upload.name, + "j": raw_upload.job_name, + "u": raw_upload.ci_run_url, + "p": raw_upload.state, + "e": raw_upload.env, + "st": raw_upload.session_type, + "se": raw_upload.session_extras, }), )) } @@ -339,25 +339,60 @@ mod tests { } fn build_sample_report(path: PathBuf) -> Result { - let mut builder = SqliteReportBuilder::new(path)?; + let mut builder = SqliteReportBuilder::new_with_seed(path, 5)?; let file_1 = builder.insert_file("src/report/report.rs".to_string())?; let file_2 = builder.insert_file("src/report/models.rs".to_string())?; + let upload_1 = builder.insert_raw_upload(models::RawUpload { + timestamp: Some(123), + raw_upload_url: Some("upload 1 url".to_string()), + flags: Some(json!(["flag on upload 1"])), + provider: Some("provider upload 1".to_string()), + build: Some("build upload 1".to_string()), + name: Some("name upload 1".to_string()), + job_name: Some("job name upload 1".to_string()), + ci_run_url: Some("ci run url upload 1".to_string()), + state: Some("state upload 1".to_string()), + env: Some("env upload 1".to_string()), + session_type: Some("type upload 1".to_string()), + session_extras: Some(json!({"k1": "v1"})), + ..Default::default() + })?; + let upload_2 = builder.insert_raw_upload(models::RawUpload { + timestamp: Some(456), + raw_upload_url: Some("upload 2 url".to_string()), + flags: Some(json!(["flag on upload 2"])), + provider: Some("provider upload 2".to_string()), + build: Some("build upload 2".to_string()), + name: Some("name upload 2".to_string()), + job_name: Some("job name upload 2".to_string()), + ci_run_url: Some("ci run url upload 2".to_string()), + state: Some("state upload 2".to_string()), + env: Some("env upload 2".to_string()), + session_type: Some("type upload 2".to_string()), + session_extras: Some(json!({"k2": "v2"})), + ..Default::default() + })?; + let line_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 1, coverage_type: models::CoverageType::Line, hits: Some(3), ..Default::default() })?; + let line_2 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 1, coverage_type: models::CoverageType::Line, hits: Some(4), ..Default::default() })?; - let line_3 = builder.insert_coverage_sample(models::CoverageSample { + let _line_3 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_2.id, line_no: 3, coverage_type: models::CoverageType::Line, @@ -366,6 +401,7 @@ mod tests { })?; let branch_sample_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 3, coverage_type: models::CoverageType::Branch, @@ -374,16 +410,18 @@ mod tests { ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_1.source_file_id, - sample_id: branch_sample_1.id, + local_sample_id: branch_sample_1.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "0:jump".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_1.source_file_id, - sample_id: branch_sample_1.id, + local_sample_id: branch_sample_1.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "1".to_string(), @@ -391,6 +429,7 @@ mod tests { })?; let branch_sample_2 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 6, coverage_type: models::CoverageType::Branch, @@ -399,32 +438,36 @@ mod tests { ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "0:jump".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 1, branch_format: models::BranchFormat::Condition, branch: "1".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 0, branch_format: models::BranchFormat::Condition, branch: "2".to_string(), ..Default::default() })?; let _ = builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_1.id, source_file_id: branch_sample_2.source_file_id, - sample_id: branch_sample_2.id, + local_sample_id: branch_sample_2.local_sample_id, hits: 0, branch_format: models::BranchFormat::Condition, branch: "3".to_string(), @@ -432,6 +475,7 @@ mod tests { })?; let method_sample_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -439,8 +483,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_method_data(models::MethodData { + raw_upload_id: upload_1.id, source_file_id: method_sample_1.source_file_id, - sample_id: Some(method_sample_1.id), + local_sample_id: method_sample_1.local_sample_id, line_no: Some(method_sample_1.line_no), hit_branches: Some(2), total_branches: Some(4), @@ -450,6 +495,7 @@ mod tests { })?; let method_sample_2 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -457,8 +503,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_method_data(models::MethodData { + raw_upload_id: upload_1.id, source_file_id: method_sample_2.source_file_id, - sample_id: Some(method_sample_2.id), + local_sample_id: method_sample_2.local_sample_id, line_no: Some(method_sample_2.line_no), hit_branches: Some(2), total_branches: Some(4), @@ -466,6 +513,7 @@ mod tests { })?; let method_sample_3 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_2.id, line_no: 5, coverage_type: models::CoverageType::Method, @@ -473,8 +521,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_method_data(models::MethodData { + raw_upload_id: upload_2.id, source_file_id: method_sample_3.source_file_id, - sample_id: Some(method_sample_3.id), + local_sample_id: method_sample_3.local_sample_id, line_no: Some(method_sample_3.line_no), hit_complexity_paths: Some(2), total_complexity: Some(4), @@ -482,6 +531,7 @@ mod tests { })?; let line_with_partial_1 = builder.insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 8, coverage_type: models::CoverageType::Line, @@ -489,8 +539,9 @@ mod tests { ..Default::default() })?; let _ = builder.insert_span_data(models::SpanData { + raw_upload_id: upload_1.id, source_file_id: line_with_partial_1.source_file_id, - sample_id: Some(line_with_partial_1.id), + local_sample_id: Some(line_with_partial_1.local_sample_id), start_line: Some(line_with_partial_1.line_no), start_col: Some(3), end_line: Some(line_with_partial_1.line_no), @@ -499,97 +550,17 @@ mod tests { ..Default::default() })?; - let upload_1 = builder.insert_context(models::ContextType::Upload, "codecov-rs CI")?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(line_1.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(line_2.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(branch_sample_1.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(branch_sample_2.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(method_sample_1.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(method_sample_2.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_1.id, - sample_id: Some(line_with_partial_1.id), - ..Default::default() - })?; - - let upload_2 = builder.insert_context(models::ContextType::Upload, "codecov-rs CI 2")?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_2.id, - sample_id: Some(line_3.id), - ..Default::default() - })?; - let _ = builder.associate_context(models::ContextAssoc { - context_id: upload_2.id, - sample_id: Some(method_sample_3.id), - ..Default::default() - })?; - let label_1 = builder.insert_context(models::ContextType::TestCase, "test-case")?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_1.id, - sample_id: Some(line_1.id), + raw_upload_id: upload_1.id, + local_sample_id: Some(line_1.local_sample_id), ..Default::default() })?; let _ = builder.associate_context(models::ContextAssoc { context_id: label_1.id, - sample_id: Some(line_2.id), - ..Default::default() - })?; - - let _ = builder.insert_upload_details(models::UploadDetails { - context_id: upload_1.id, - timestamp: Some(123), - raw_upload_url: Some("upload 1 url".to_string()), - flags: Some(json!(["flag on upload 1"])), - provider: Some("provider upload 1".to_string()), - build: Some("build upload 1".to_string()), - name: Some("name upload 1".to_string()), - job_name: Some("job name upload 1".to_string()), - ci_run_url: Some("ci run url upload 1".to_string()), - state: Some("state upload 1".to_string()), - env: Some("env upload 1".to_string()), - session_type: Some("type upload 1".to_string()), - session_extras: Some(json!({"k1": "v1"})), - ..Default::default() - })?; - let _ = builder.insert_upload_details(models::UploadDetails { - context_id: upload_2.id, - timestamp: Some(456), - raw_upload_url: Some("upload 2 url".to_string()), - flags: Some(json!(["flag on upload 2"])), - provider: Some("provider upload 2".to_string()), - build: Some("build upload 2".to_string()), - name: Some("name upload 2".to_string()), - job_name: Some("job name upload 2".to_string()), - ci_run_url: Some("ci run url upload 2".to_string()), - state: Some("state upload 2".to_string()), - env: Some("env upload 2".to_string()), - session_type: Some("type upload 2".to_string()), - session_extras: Some(json!({"k2": "v2"})), + raw_upload_id: upload_1.id, + local_sample_id: Some(line_2.local_sample_id), ..Default::default() })?; diff --git a/src/report/sqlite/mod.rs b/src/report/sqlite/mod.rs index 45edd72..c264dad 100644 --- a/src/report/sqlite/mod.rs +++ b/src/report/sqlite/mod.rs @@ -5,7 +5,6 @@ * Notes on SQLite performance: * - Some `ORDER BY` clauses are to make writing test cases simple and may * not be necessary - * - Tables with UUID/BLOB PKs are declared [`WITHOUT ROWID`](https://www.sqlite.org/withoutrowid.html) */ use std::path::PathBuf; diff --git a/src/report/sqlite/models.rs b/src/report/sqlite/models.rs index f5ccb90..986d5ef 100644 --- a/src/report/sqlite/models.rs +++ b/src/report/sqlite/models.rs @@ -25,8 +25,6 @@ use crate::{error::Result, parsers::json::JsonVal}; /// - `fn param_bindings(&self) -> [&dyn rusqlite::ToSql; FIELD_COUNT]`: a /// function which returns an array of `ToSql` trait objects that should bind /// to each of the `?`s in `INSERT_PLACEHOLDER`. -/// - `fn assign_id(&mut self)`: a function which generates and sets an -/// appropriate ID for the model. /// /// Example: /// ``` @@ -46,10 +44,6 @@ use crate::{error::Result, parsers::json::JsonVal}; /// &self.path as &dyn rusqlite::ToSql, /// ] /// } -/// -/// fn assign_id(&mut self) { -/// self.id = seahash::hash(self.path.as_bytes()) as i64; -/// } /// } /// ``` /// @@ -62,8 +56,6 @@ pub trait Insertable { fn param_bindings(&self) -> [&dyn rusqlite::ToSql; FIELD_COUNT]; - fn assign_id(&mut self); - fn insert(model: &Self, conn: &rusqlite::Connection) -> Result<()> { let mut stmt = conn.prepare_cached( // Maybe turn this in to a lazily-initialized static @@ -163,10 +155,6 @@ impl Insertable<2> for SourceFile { &self.path as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = seahash::hash(self.path.as_bytes()) as i64; - } } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for CoverageSample { @@ -174,7 +162,8 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for CoverageSample { fn try_from(row: &'a ::rusqlite::Row) -> Result { Ok(Self { - id: row.get(row.as_ref().column_index("id")?)?, + raw_upload_id: row.get(row.as_ref().column_index("raw_upload_id")?)?, + local_sample_id: row.get(row.as_ref().column_index("local_sample_id")?)?, source_file_id: row.get(row.as_ref().column_index("source_file_id")?)?, line_no: row.get(row.as_ref().column_index("line_no")?)?, coverage_type: row.get(row.as_ref().column_index("coverage_type")?)?, @@ -185,13 +174,14 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for CoverageSample { } } -impl Insertable<7> for CoverageSample { - const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO coverage_sample (id, source_file_id, line_no, coverage_type, hits, hit_branches, total_branches) VALUES "; - const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?)"; +impl Insertable<8> for CoverageSample { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO coverage_sample (raw_upload_id, local_sample_id, source_file_id, line_no, coverage_type, hits, hit_branches, total_branches) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?)"; - fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 7] { + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 8] { [ - &self.id as &dyn rusqlite::ToSql, + &self.raw_upload_id as &dyn rusqlite::ToSql, + &self.local_sample_id as &dyn rusqlite::ToSql, &self.source_file_id as &dyn rusqlite::ToSql, &self.line_no as &dyn rusqlite::ToSql, &self.coverage_type as &dyn rusqlite::ToSql, @@ -200,10 +190,6 @@ impl Insertable<7> for CoverageSample { &self.total_branches as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = uuid::Uuid::new_v4(); - } } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for BranchesData { @@ -211,9 +197,10 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for BranchesData { fn try_from(row: &'a ::rusqlite::Row) -> Result { Ok(Self { - id: row.get(row.as_ref().column_index("id")?)?, + raw_upload_id: row.get(row.as_ref().column_index("raw_upload_id")?)?, + local_branch_id: row.get(row.as_ref().column_index("local_branch_id")?)?, source_file_id: row.get(row.as_ref().column_index("source_file_id")?)?, - sample_id: row.get(row.as_ref().column_index("sample_id")?)?, + local_sample_id: row.get(row.as_ref().column_index("local_sample_id")?)?, hits: row.get(row.as_ref().column_index("hits")?)?, branch_format: row.get(row.as_ref().column_index("branch_format")?)?, branch: row.get(row.as_ref().column_index("branch")?)?, @@ -221,24 +208,21 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for BranchesData { } } -impl Insertable<6> for BranchesData { - const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO branches_data (id, source_file_id, sample_id, hits, branch_format, branch) VALUES "; - const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?)"; +impl Insertable<7> for BranchesData { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO branches_data (raw_upload_id, local_branch_id, source_file_id, local_sample_id, hits, branch_format, branch) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?)"; - fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 6] { + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 7] { [ - &self.id as &dyn rusqlite::ToSql, + &self.raw_upload_id as &dyn rusqlite::ToSql, + &self.local_branch_id as &dyn rusqlite::ToSql, &self.source_file_id as &dyn rusqlite::ToSql, - &self.sample_id as &dyn rusqlite::ToSql, + &self.local_sample_id as &dyn rusqlite::ToSql, &self.hits as &dyn rusqlite::ToSql, &self.branch_format as &dyn rusqlite::ToSql, &self.branch as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = uuid::Uuid::new_v4(); - } } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for MethodData { @@ -246,9 +230,10 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for MethodData { fn try_from(row: &'a ::rusqlite::Row) -> Result { Ok(Self { - id: row.get(row.as_ref().column_index("id")?)?, + raw_upload_id: row.get(row.as_ref().column_index("raw_upload_id")?)?, + local_method_id: row.get(row.as_ref().column_index("local_method_id")?)?, source_file_id: row.get(row.as_ref().column_index("source_file_id")?)?, - sample_id: row.get(row.as_ref().column_index("sample_id")?)?, + local_sample_id: row.get(row.as_ref().column_index("local_sample_id")?)?, line_no: row.get(row.as_ref().column_index("line_no")?)?, hit_branches: row.get(row.as_ref().column_index("hit_branches")?)?, total_branches: row.get(row.as_ref().column_index("total_branches")?)?, @@ -258,15 +243,16 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for MethodData { } } -impl Insertable<8> for MethodData { - const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO method_data (id, source_file_id, sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity) VALUES "; - const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?)"; +impl Insertable<9> for MethodData { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO method_data (raw_upload_id, local_method_id, source_file_id, local_sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?, ?)"; - fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 8] { + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 9] { [ - &self.id as &dyn rusqlite::ToSql, + &self.raw_upload_id as &dyn rusqlite::ToSql, + &self.local_method_id as &dyn rusqlite::ToSql, &self.source_file_id as &dyn rusqlite::ToSql, - &self.sample_id as &dyn rusqlite::ToSql, + &self.local_sample_id as &dyn rusqlite::ToSql, &self.line_no as &dyn rusqlite::ToSql, &self.hit_branches as &dyn rusqlite::ToSql, &self.total_branches as &dyn rusqlite::ToSql, @@ -274,10 +260,6 @@ impl Insertable<8> for MethodData { &self.total_complexity as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = uuid::Uuid::new_v4(); - } } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for SpanData { @@ -285,9 +267,10 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for SpanData { fn try_from(row: &'a ::rusqlite::Row) -> Result { Ok(Self { - id: row.get(row.as_ref().column_index("id")?)?, + raw_upload_id: row.get(row.as_ref().column_index("raw_upload_id")?)?, + local_span_id: row.get(row.as_ref().column_index("local_span_id")?)?, source_file_id: row.get(row.as_ref().column_index("source_file_id")?)?, - sample_id: row.get(row.as_ref().column_index("sample_id")?)?, + local_sample_id: row.get(row.as_ref().column_index("local_sample_id")?)?, hits: row.get(row.as_ref().column_index("hits")?)?, start_line: row.get(row.as_ref().column_index("start_line")?)?, start_col: row.get(row.as_ref().column_index("start_col")?)?, @@ -297,15 +280,16 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for SpanData { } } -impl Insertable<8> for SpanData { - const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO span_data (id, source_file_id, sample_id, hits, start_line, start_col, end_line, end_col) VALUES "; - const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?)"; +impl Insertable<9> for SpanData { + const INSERT_QUERY_PRELUDE: &'static str = "INSERT INTO span_data (raw_upload_id, local_span_id, source_file_id, local_sample_id, hits, start_line, start_col, end_line, end_col) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?, ?, ?, ?, ?)"; - fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 8] { + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 9] { [ - &self.id as &dyn rusqlite::ToSql, + &self.raw_upload_id as &dyn rusqlite::ToSql, + &self.local_span_id as &dyn rusqlite::ToSql, &self.source_file_id as &dyn rusqlite::ToSql, - &self.sample_id as &dyn rusqlite::ToSql, + &self.local_sample_id as &dyn rusqlite::ToSql, &self.hits as &dyn rusqlite::ToSql, &self.start_line as &dyn rusqlite::ToSql, &self.start_col as &dyn rusqlite::ToSql, @@ -313,10 +297,6 @@ impl Insertable<8> for SpanData { &self.end_col as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = uuid::Uuid::new_v4(); - } } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for ContextAssoc { @@ -325,30 +305,26 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for ContextAssoc { fn try_from(row: &'a ::rusqlite::Row) -> Result { Ok(Self { context_id: row.get(row.as_ref().column_index("context_id")?)?, - sample_id: row.get(row.as_ref().column_index("sample_id")?)?, - branch_id: row.get(row.as_ref().column_index("branch_id")?)?, - method_id: row.get(row.as_ref().column_index("method_id")?)?, - span_id: row.get(row.as_ref().column_index("span_id")?)?, + raw_upload_id: row.get(row.as_ref().column_index("raw_upload_id")?)?, + local_sample_id: row.get(row.as_ref().column_index("local_sample_id")?)?, + local_span_id: row.get(row.as_ref().column_index("local_span_id")?)?, }) } } -impl Insertable<5> for ContextAssoc { +impl Insertable<4> for ContextAssoc { const INSERT_QUERY_PRELUDE: &'static str = - "INSERT INTO context_assoc (context_id, sample_id, branch_id, method_id, span_id) VALUES "; - const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?, ?)"; + "INSERT INTO context_assoc (context_id, raw_upload_id, local_sample_id, local_span_id) VALUES "; + const INSERT_PLACEHOLDER: &'static str = "(?, ?, ?, ?)"; - fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 5] { + fn param_bindings(&self) -> [&dyn rusqlite::ToSql; 4] { [ &self.context_id as &dyn rusqlite::ToSql, - &self.sample_id as &dyn rusqlite::ToSql, - &self.branch_id as &dyn rusqlite::ToSql, - &self.method_id as &dyn rusqlite::ToSql, - &self.span_id as &dyn rusqlite::ToSql, + &self.raw_upload_id as &dyn rusqlite::ToSql, + &self.local_sample_id as &dyn rusqlite::ToSql, + &self.local_span_id as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) {} } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for Context { @@ -375,13 +351,9 @@ impl Insertable<3> for Context { &self.name as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = seahash::hash(self.name.as_bytes()) as i64; - } } -impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for UploadDetails { +impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for RawUpload { type Error = rusqlite::Error; fn try_from(row: &'a ::rusqlite::Row) -> Result { @@ -398,7 +370,7 @@ impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for UploadDetails { None }; Ok(Self { - context_id: row.get(row.as_ref().column_index("context_id")?)?, + id: row.get(row.as_ref().column_index("id")?)?, timestamp: row.get(row.as_ref().column_index("timestamp")?)?, raw_upload_url: row.get(row.as_ref().column_index("raw_upload_url")?)?, flags, @@ -451,7 +423,10 @@ mod tests { use tempfile::TempDir; use super::{ - super::{super::Report, SqliteReport}, + super::{ + super::{Report, ReportBuilder}, + SqliteReport, SqliteReportBuilder, + }, *, }; @@ -471,10 +446,6 @@ mod tests { &self.data as &dyn rusqlite::ToSql, ] } - - fn assign_id(&mut self) { - self.id = seahash::hash(self.data.as_bytes()) as i64; - } } impl<'a> std::convert::TryFrom<&'a rusqlite::Row<'a>> for TestModel { @@ -489,7 +460,7 @@ mod tests { } struct Ctx { - _temp_dir: TempDir, + temp_dir: TempDir, report: SqliteReport, } @@ -506,10 +477,7 @@ mod tests { ) .unwrap(); - Ctx { - _temp_dir: temp_dir, - report, - } + Ctx { temp_dir, report } } fn list_test_models(report: &SqliteReport) -> Vec { @@ -576,7 +544,7 @@ mod tests { let model = Context { id: 0, - context_type: ContextType::Upload, + context_type: ContextType::TestCase, name: "test_upload".to_string(), }; @@ -596,103 +564,120 @@ mod tests { #[test] fn test_context_assoc_single_insert() { let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); + + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let context = report_builder + .insert_context(ContextType::TestCase, "foo") + .unwrap(); + + let report = report_builder.build().unwrap(); let model = ContextAssoc { - context_id: 0, - sample_id: Some(uuid::Uuid::new_v4()), + context_id: context.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(rand::random()), + local_span_id: None, ..Default::default() }; - >::insert(&model, &ctx.report.conn).unwrap(); - let assoc: ContextAssoc = ctx - .report + >::insert(&model, &report.conn).unwrap(); + let assoc: ContextAssoc = report .conn .query_row( - "SELECT context_id, sample_id, branch_id, method_id, span_id FROM context_assoc", + "SELECT context_id, raw_upload_id, local_sample_id, local_span_id FROM context_assoc", [], |row| row.try_into(), ) .unwrap(); assert_eq!(assoc, model); - let duplicate_result = >::insert(&model, &ctx.report.conn); + /* TODO: Figure out how to re-enable this part of the test + let duplicate_result = >::insert(&model, &ctx.report.conn); let error = duplicate_result.unwrap_err(); assert_eq!( error.to_string(), - "sqlite failure: 'UNIQUE constraint failed: context_assoc.context_id, context_assoc.sample_id'" + "sqlite failure: 'UNIQUE constraint failed: context_assoc.context_id, context_assoc.raw_upload_id, context_assoc.local_sample_id'" ); + */ } #[test] fn test_coverage_sample_single_insert() { let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); - >::insert( - &SourceFile { - id: 0, - ..Default::default() - }, - &ctx.report.conn, - ) - .unwrap(); + let source_file = report_builder.insert_file("foo.rs".to_string()).unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + + let report = report_builder.build().unwrap(); let model = CoverageSample { - id: uuid::Uuid::new_v4(), - source_file_id: 0, + raw_upload_id: raw_upload.id, + local_sample_id: rand::random(), + source_file_id: source_file.id, ..Default::default() }; - >::insert(&model, &ctx.report.conn).unwrap(); - let duplicate_result = >::insert(&model, &ctx.report.conn); + >::insert(&model, &report.conn).unwrap(); + let duplicate_result = >::insert(&model, &report.conn); - let samples = ctx.report.list_coverage_samples().unwrap(); + let samples = report.list_coverage_samples().unwrap(); assert_eq!(samples, vec![model]); let error = duplicate_result.unwrap_err(); assert_eq!( error.to_string(), - "sqlite failure: 'UNIQUE constraint failed: coverage_sample.id'" + "sqlite failure: 'UNIQUE constraint failed: coverage_sample.raw_upload_id, coverage_sample.local_sample_id'" ); } #[test] fn test_branches_data_single_insert() { let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); - >::insert( - &SourceFile { - id: 0, - ..Default::default() - }, - &ctx.report.conn, - ) - .unwrap(); + let source_file = report_builder.insert_file("path".to_string()).unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); - let sample_id = uuid::Uuid::new_v4(); - >::insert( + let report = report_builder.build().unwrap(); + + let local_sample_id = rand::random(); + >::insert( &CoverageSample { - id: sample_id, - source_file_id: 0, + raw_upload_id: raw_upload.id, + local_sample_id, + source_file_id: source_file.id, ..Default::default() }, - &ctx.report.conn, + &report.conn, ) .unwrap(); let model = BranchesData { - id: uuid::Uuid::new_v4(), - sample_id, - source_file_id: 0, + raw_upload_id: raw_upload.id, + local_branch_id: rand::random(), + local_sample_id, + source_file_id: source_file.id, ..Default::default() }; - >::insert(&model, &ctx.report.conn).unwrap(); - let duplicate_result = >::insert(&model, &ctx.report.conn); + >::insert(&model, &report.conn).unwrap(); + let duplicate_result = >::insert(&model, &report.conn); - let branch: BranchesData = ctx.report + let branch: BranchesData = report .conn .query_row( - "SELECT id, source_file_id, sample_id, hits, branch_format, branch FROM branches_data", + "SELECT local_branch_id, source_file_id, local_sample_id, raw_upload_id, hits, branch_format, branch FROM branches_data", [], |row| row.try_into(), ).unwrap(); @@ -701,36 +686,45 @@ mod tests { let error = duplicate_result.unwrap_err(); assert_eq!( error.to_string(), - "sqlite failure: 'UNIQUE constraint failed: branches_data.id'" + "sqlite failure: 'UNIQUE constraint failed: branches_data.raw_upload_id, branches_data.local_branch_id'" ); } #[test] fn test_method_data_single_insert() { let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); - >::insert( - &SourceFile { - id: 0, + let source_file = report_builder.insert_file("foo.rs".to_string()).unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let coverage_sample = report_builder + .insert_coverage_sample(CoverageSample { + raw_upload_id: raw_upload.id, + source_file_id: source_file.id, ..Default::default() - }, - &ctx.report.conn, - ) - .unwrap(); + }) + .unwrap(); + + let report = report_builder.build().unwrap(); let model = MethodData { - id: uuid::Uuid::new_v4(), - source_file_id: 0, + raw_upload_id: raw_upload.id, + local_method_id: rand::random(), + local_sample_id: coverage_sample.local_sample_id, + source_file_id: source_file.id, ..Default::default() }; - >::insert(&model, &ctx.report.conn).unwrap(); - let duplicate_result = >::insert(&model, &ctx.report.conn); + >::insert(&model, &report.conn).unwrap(); + let duplicate_result = >::insert(&model, &report.conn); - let method: MethodData = ctx.report + let method: MethodData = report .conn .query_row( - "SELECT id, source_file_id, sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity FROM method_data", + "SELECT raw_upload_id, local_method_id, source_file_id, local_sample_id, line_no, hit_branches, total_branches, hit_complexity_paths, total_complexity FROM method_data", [], |row| row.try_into(), ).unwrap(); @@ -739,36 +733,37 @@ mod tests { let error = duplicate_result.unwrap_err(); assert_eq!( error.to_string(), - "sqlite failure: 'UNIQUE constraint failed: method_data.id'" + "sqlite failure: 'UNIQUE constraint failed: method_data.raw_upload_id, method_data.local_method_id'" ); } #[test] fn test_span_data_single_insert() { let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); - >::insert( - &SourceFile { - id: 0, - ..Default::default() - }, - &ctx.report.conn, - ) - .unwrap(); + let source_file = report_builder.insert_file("foo.rs".to_string()).unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + + let report = report_builder.build().unwrap(); let model = SpanData { - id: uuid::Uuid::new_v4(), - source_file_id: 0, + raw_upload_id: raw_upload.id, + local_span_id: rand::random(), + source_file_id: source_file.id, ..Default::default() }; - >::insert(&model, &ctx.report.conn).unwrap(); - let duplicate_result = >::insert(&model, &ctx.report.conn); + >::insert(&model, &report.conn).unwrap(); + let duplicate_result = >::insert(&model, &report.conn); - let branch: SpanData = ctx.report + let branch: SpanData = report .conn .query_row( - "SELECT id, source_file_id, sample_id, hits, start_line, start_col, end_line, end_col FROM span_data", + "SELECT raw_upload_id, local_span_id, source_file_id, local_sample_id, hits, start_line, start_col, end_line, end_col FROM span_data", [], |row| row.try_into(), ).unwrap(); @@ -777,7 +772,7 @@ mod tests { let error = duplicate_result.unwrap_err(); assert_eq!( error.to_string(), - "sqlite failure: 'UNIQUE constraint failed: span_data.id'" + "sqlite failure: 'UNIQUE constraint failed: span_data.raw_upload_id, span_data.local_span_id'" ); } } diff --git a/src/report/sqlite/queries/totals.sql b/src/report/sqlite/queries/totals.sql index 75ef438..5db9b0b 100644 --- a/src/report/sqlite/queries/totals.sql +++ b/src/report/sqlite/queries/totals.sql @@ -2,9 +2,7 @@ with uploads as ( select count(*) as count from - context -where - context.context_type = 'Upload' + raw_upload ), test_cases as ( select @@ -38,4 +36,5 @@ from left join method_data on - coverage_sample.id = method_data.sample_id + coverage_sample.raw_upload_id = method_data.raw_upload_id + and coverage_sample.local_sample_id = method_data.local_sample_id diff --git a/src/report/sqlite/report.rs b/src/report/sqlite/report.rs index fc00906..a75c316 100644 --- a/src/report/sqlite/report.rs +++ b/src/report/sqlite/report.rs @@ -47,7 +47,7 @@ impl Report for SqliteReport { fn list_coverage_samples(&self) -> Result> { let mut stmt = self .conn - .prepare_cached("SELECT id, source_file_id, line_no, coverage_type, hits, hit_branches, total_branches FROM coverage_sample ORDER BY 2, 3")?; + .prepare_cached("SELECT raw_upload_id, local_sample_id, source_file_id, line_no, coverage_type, hits, hit_branches, total_branches FROM coverage_sample ORDER BY 2, 3")?; let samples = stmt .query_map([], |row| row.try_into())? .collect::>>()?; @@ -63,7 +63,7 @@ impl Report for SqliteReport { .conn .prepare_cached("SELECT context.id, context.context_type, context.name FROM context INNER JOIN context_assoc ON context.id = context_assoc.context_id WHERE context_assoc.sample_id = ?1")?; let contexts = stmt - .query_map([sample.id], |row| row.try_into())? + .query_map([sample.local_sample_id], |row| row.try_into())? .collect::>>()?; Ok(contexts) } @@ -75,23 +75,26 @@ impl Report for SqliteReport { ) -> Result> { let mut stmt = self .conn - .prepare_cached("SELECT sample.id, sample.source_file_id, sample.line_no, sample.coverage_type, sample.hits, sample.hit_branches, sample.total_branches FROM coverage_sample sample INNER JOIN source_file ON sample.source_file_id = source_file.id WHERE source_file_id=?1")?; + .prepare_cached("SELECT sample.local_sample_id, sample.raw_upload_id, sample.source_file_id, sample.line_no, sample.coverage_type, sample.hits, sample.hit_branches, sample.total_branches FROM coverage_sample sample INNER JOIN source_file ON sample.source_file_id = source_file.id WHERE source_file_id=?1")?; let samples = stmt .query_map([file.id], |row| row.try_into())? .collect::>>()?; Ok(samples) } - fn get_details_for_upload(&self, upload: &models::Context) -> Result { - assert_eq!(upload.context_type, models::ContextType::Upload); - let mut stmt = self.conn.prepare_cached("SELECT context_id, timestamp, raw_upload_url, flags, provider, build, name, job_name, ci_run_url, state, env, session_type, session_extras FROM upload_details WHERE context_id = ?1")?; - Ok(stmt.query_row([upload.id], |row| row.try_into())?) + fn list_raw_uploads(&self) -> Result> { + let mut stmt = self.conn.prepare_cached("SELECT id, timestamp, raw_upload_url, flags, provider, build, name, job_name, ci_run_url, state, env, session_type, session_extras FROM raw_upload")?; + let uploads = stmt + .query_map([], |row| row.try_into())? + .collect::>>()?; + Ok(uploads) } /// Merge `other` into `self` without modifying `other`. /// /// TODO: Probably put this in a commit fn merge(&mut self, other: &SqliteReport) -> Result<()> { + // let tx = self.conn.transaction()?; let _ = self .conn .execute("ATTACH DATABASE ?1 AS other", [other.conn.path()])?; @@ -101,8 +104,10 @@ impl Report for SqliteReport { // use a hash of their "names" as their PK so any instance of them will // come up with the same PK. We can `INSERT OR IGNORE` to effectively union the tables "INSERT OR IGNORE INTO source_file SELECT * FROM other.source_file", + "INSERT OR IGNORE INTO raw_upload SELECT * FROM other.raw_upload", "INSERT OR IGNORE INTO context SELECT * FROM other.context", - // For everything else, we use UUIDs as IDs and can simply concatenate the tables + // For everything else, we use a joint primary key that should be globally unique and + // can simply concatenate the tables "INSERT INTO coverage_sample SELECT * FROM other.coverage_sample", "INSERT INTO branches_data SELECT * FROM other.branches_data", "INSERT INTO method_data SELECT * FROM other.method_data", @@ -166,19 +171,23 @@ mod tests { let db_file_left = ctx.temp_dir.path().join("left.sqlite"); let db_file_right = ctx.temp_dir.path().join("right.sqlite"); - let mut left_report_builder = SqliteReportBuilder::new(db_file_left).unwrap(); + let mut left_report_builder = SqliteReportBuilder::new_with_seed(db_file_left, 5).unwrap(); let file_1 = left_report_builder .insert_file("src/report.rs".to_string()) .unwrap(); let file_2 = left_report_builder .insert_file("src/report/models.rs".to_string()) .unwrap(); - let context_1 = left_report_builder - .insert_context(models::ContextType::Upload, "codecov-rs CI") + let upload_1 = left_report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let test_case_1 = left_report_builder + .insert_context(models::ContextType::TestCase, "test case 1") .unwrap(); let line_1 = left_report_builder .insert_coverage_sample(models::CoverageSample { source_file_id: file_1.id, + raw_upload_id: upload_1.id, line_no: 1, coverage_type: models::CoverageType::Line, ..Default::default() @@ -186,6 +195,7 @@ mod tests { .unwrap(); let line_2 = left_report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 1, coverage_type: models::CoverageType::Branch, @@ -196,6 +206,7 @@ mod tests { .unwrap(); let line_3 = left_report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -205,24 +216,30 @@ mod tests { .unwrap(); for line in [&line_1, &line_2, &line_3] { let _ = left_report_builder.associate_context(models::ContextAssoc { - context_id: context_1.id, - sample_id: Some(line.id), + context_id: test_case_1.id, + raw_upload_id: upload_1.id, + local_sample_id: Some(line.local_sample_id), ..Default::default() }); } - let mut right_report_builder = SqliteReportBuilder::new(db_file_right).unwrap(); + let mut right_report_builder = + SqliteReportBuilder::new_with_seed(db_file_right, 10).unwrap(); let file_2 = right_report_builder .insert_file("src/report/models.rs".to_string()) .unwrap(); let file_3 = right_report_builder .insert_file("src/report/schema.rs".to_string()) .unwrap(); - let context_2 = right_report_builder - .insert_context(models::ContextType::Upload, "codecov-rs CI 2") + let upload_2 = right_report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let test_case_2 = right_report_builder + .insert_context(models::ContextType::TestCase, "test case 2") .unwrap(); let line_4 = right_report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_2.id, line_no: 3, coverage_type: models::CoverageType::Line, @@ -232,6 +249,7 @@ mod tests { .unwrap(); let line_5 = right_report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_3.id, line_no: 1, coverage_type: models::CoverageType::Branch, @@ -241,8 +259,9 @@ mod tests { }) .unwrap(); let _ = right_report_builder.insert_branches_data(models::BranchesData { + raw_upload_id: upload_2.id, source_file_id: file_2.id, - sample_id: line_5.id, + local_sample_id: line_5.local_sample_id, hits: 0, branch_format: models::BranchFormat::Condition, branch: "1".to_string(), @@ -250,6 +269,7 @@ mod tests { }); let line_6 = right_report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_2.id, source_file_id: file_2.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -258,8 +278,9 @@ mod tests { }) .unwrap(); let _ = right_report_builder.insert_method_data(models::MethodData { + raw_upload_id: upload_2.id, source_file_id: file_2.id, - sample_id: Some(line_6.id), + local_sample_id: line_6.local_sample_id, line_no: Some(2), hit_complexity_paths: Some(1), total_complexity: Some(2), @@ -267,8 +288,8 @@ mod tests { }); for line in [&line_4, &line_5, &line_6] { let _ = right_report_builder.associate_context(models::ContextAssoc { - context_id: context_2.id, - sample_id: Some(line.id), + context_id: test_case_2.id, + local_sample_id: Some(line.local_sample_id), ..Default::default() }); } @@ -282,29 +303,32 @@ mod tests { ); assert_eq!( left.list_contexts().unwrap().sort_by_key(|c| c.id), - [&context_1, &context_2].sort_by_key(|c| c.id), + [&test_case_1, &test_case_2].sort_by_key(|c| c.id), ); assert_eq!( - left.list_coverage_samples().unwrap().sort_by_key(|s| s.id), - [&line_1, &line_2, &line_3, &line_4, &line_5, &line_6].sort_by_key(|s| s.id), + left.list_coverage_samples() + .unwrap() + .sort_by_key(|s| s.local_sample_id), + [&line_1, &line_2, &line_3, &line_4, &line_5, &line_6] + .sort_by_key(|s| s.local_sample_id), ); assert_eq!( left.list_samples_for_file(&file_1) .unwrap() - .sort_by_key(|s| s.id), - [&line_1].sort_by_key(|s| s.id), + .sort_by_key(|s| s.local_sample_id), + [&line_1].sort_by_key(|s| s.local_sample_id), ); assert_eq!( left.list_samples_for_file(&file_2) .unwrap() - .sort_by_key(|s| s.id), - [&line_2, &line_3, &line_4].sort_by_key(|s| s.id), + .sort_by_key(|s| s.local_sample_id), + [&line_2, &line_3, &line_4].sort_by_key(|s| s.local_sample_id), ); assert_eq!( left.list_samples_for_file(&file_3) .unwrap() - .sort_by_key(|s| s.id), - [&line_5, &line_6].sort_by_key(|s| s.id), + .sort_by_key(|s| s.local_sample_id), + [&line_5, &line_6].sort_by_key(|s| s.local_sample_id), ); } @@ -321,14 +345,15 @@ mod tests { let file_2 = report_builder .insert_file("src/report/models.rs".to_string()) .unwrap(); - let context_1 = report_builder - .insert_context(models::ContextType::Upload, "codecov-rs CI") + let upload_1 = report_builder + .insert_raw_upload(Default::default()) .unwrap(); - let context_2 = report_builder + let test_case_1 = report_builder .insert_context(models::ContextType::TestCase, "test_totals") .unwrap(); let line_1 = report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_1.id, line_no: 1, coverage_type: models::CoverageType::Line, @@ -338,6 +363,7 @@ mod tests { let line_2 = report_builder .insert_coverage_sample(models::CoverageSample { source_file_id: file_2.id, + raw_upload_id: upload_1.id, line_no: 1, coverage_type: models::CoverageType::Branch, hit_branches: Some(1), @@ -347,6 +373,7 @@ mod tests { .unwrap(); let line_3 = report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: upload_1.id, source_file_id: file_2.id, line_no: 2, coverage_type: models::CoverageType::Method, @@ -355,8 +382,9 @@ mod tests { }) .unwrap(); let _ = report_builder.insert_method_data(models::MethodData { + raw_upload_id: upload_1.id, source_file_id: file_2.id, - sample_id: Some(line_3.id), + local_sample_id: line_3.local_sample_id, line_no: Some(2), hit_complexity_paths: Some(2), total_complexity: Some(4), @@ -364,13 +392,9 @@ mod tests { }); for line in [&line_1, &line_2, &line_3] { let _ = report_builder.associate_context(models::ContextAssoc { - context_id: context_1.id, - sample_id: Some(line.id), - ..Default::default() - }); - let _ = report_builder.associate_context(models::ContextAssoc { - context_id: context_2.id, - sample_id: Some(line.id), + raw_upload_id: upload_1.id, + context_id: test_case_1.id, + local_sample_id: Some(line.local_sample_id), ..Default::default() }); } diff --git a/src/report/sqlite/report_builder.rs b/src/report/sqlite/report_builder.rs index 30c75c1..7af8d98 100644 --- a/src/report/sqlite/report_builder.rs +++ b/src/report/sqlite/report_builder.rs @@ -1,5 +1,9 @@ -use std::path::{Path, PathBuf}; +use std::{ + ops::RangeFrom, + path::{Path, PathBuf}, +}; +use rand::{rngs::StdRng, Rng, SeedableRng}; use rusqlite::{Connection, Transaction}; use super::{models::Insertable, open_database, SqliteReport}; @@ -16,6 +20,9 @@ use crate::{ /// their `conn` member mutably borrows the SQLite database and prevents /// `build()` from moving it into a `SqliteReport`. pub struct SqliteReportBuilderTx<'a> { + id_sequence: &'a mut RangeFrom, + rng: &'a mut StdRng, + pub filename: &'a Path, pub conn: Transaction<'a>, } @@ -37,12 +44,31 @@ impl<'a> SqliteReportBuilderTx<'a> { pub struct SqliteReportBuilder { pub filename: PathBuf, pub conn: Connection, + + /// A single sequence is shared for [`CoverageSample`], [`BranchesData`], + /// [`MethodData`], and [`SpanData`]. + id_sequence: RangeFrom, + + rng: StdRng, } impl SqliteReportBuilder { - pub fn new(filename: PathBuf) -> Result { + fn new_with_rng(filename: PathBuf, rng: StdRng) -> Result { let conn = open_database(&filename)?; - Ok(SqliteReportBuilder { filename, conn }) + Ok(SqliteReportBuilder { + filename, + conn, + id_sequence: 0.., + rng, + }) + } + + pub fn new_with_seed(filename: PathBuf, seed: u64) -> Result { + Self::new_with_rng(filename, StdRng::seed_from_u64(seed)) + } + + pub fn new(filename: PathBuf) -> Result { + Self::new_with_rng(filename, StdRng::from_entropy()) } /// Create a [`SqliteReportBuilderTx`] with a [`rusqlite::Transaction`] that @@ -54,6 +80,8 @@ impl SqliteReportBuilder { let mut builder_tx = SqliteReportBuilderTx { filename: &self.filename, conn: self.conn.transaction()?, + id_sequence: &mut self.id_sequence, + rng: &mut self.rng, }; builder_tx .conn @@ -104,11 +132,8 @@ impl ReportBuilder for SqliteReportBuilder { self.transaction()?.associate_context(assoc) } - fn insert_upload_details( - &mut self, - upload_details: models::UploadDetails, - ) -> Result { - self.transaction()?.insert_upload_details(upload_details) + fn insert_raw_upload(&mut self, raw_upload: models::RawUpload) -> Result { + self.transaction()?.insert_raw_upload(raw_upload) } /// Consumes this builder and returns a [`SqliteReport`]. @@ -183,11 +208,10 @@ impl ReportBuilder for SqliteReportBuilder { impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { fn insert_file(&mut self, path: String) -> Result { - let mut model = models::SourceFile { + let model = models::SourceFile { + id: seahash::hash(path.as_bytes()) as i64, path, - ..Default::default() }; - model.assign_id(); >::insert(&model, &self.conn)?; Ok(model) } @@ -197,12 +221,11 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { context_type: models::ContextType, name: &str, ) -> Result { - let mut model = models::Context { + let model = models::Context { + id: seahash::hash(name.as_bytes()) as i64, context_type, name: name.to_string(), - ..Default::default() }; - model.assign_id(); >::insert(&model, &self.conn)?; Ok(model) } @@ -211,8 +234,9 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { &mut self, mut sample: models::CoverageSample, ) -> Result { - sample.assign_id(); - >::insert(&sample, &self.conn)?; + // TODO handle error + sample.local_sample_id = self.id_sequence.next().unwrap(); + >::insert(&sample, &self.conn)?; Ok(sample) } @@ -220,20 +244,23 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { &mut self, mut branch: models::BranchesData, ) -> Result { - branch.assign_id(); - >::insert(&branch, &self.conn)?; + // TODO handle error + branch.local_branch_id = self.id_sequence.next().unwrap(); + >::insert(&branch, &self.conn)?; Ok(branch) } fn insert_method_data(&mut self, mut method: models::MethodData) -> Result { - method.assign_id(); - >::insert(&method, &self.conn)?; + // TODO handle error + method.local_method_id = self.id_sequence.next().unwrap(); + >::insert(&method, &self.conn)?; Ok(method) } fn insert_span_data(&mut self, mut span: models::SpanData) -> Result { - span.assign_id(); - >::insert(&span, &self.conn)?; + // TODO handle error + span.local_span_id = self.id_sequence.next().unwrap(); + >::insert(&span, &self.conn)?; Ok(span) } @@ -241,35 +268,34 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { &mut self, assoc: models::ContextAssoc, ) -> Result { - >::insert(&assoc, &self.conn)?; + >::insert(&assoc, &self.conn)?; Ok(assoc) } - fn insert_upload_details( + fn insert_raw_upload( &mut self, - upload_details: models::UploadDetails, - ) -> Result { - let mut stmt = self.conn.prepare_cached("INSERT INTO upload_details (context_id, timestamp, raw_upload_url, flags, provider, build, name, job_name, ci_run_url, state, env, session_type, session_extras) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13)")?; + mut raw_upload: models::RawUpload, + ) -> Result { + let mut stmt = self.conn.prepare_cached("INSERT INTO raw_upload (id, timestamp, raw_upload_url, flags, provider, build, name, job_name, ci_run_url, state, env, session_type, session_extras) VALUES (?1, ?2, ?3, ?4, ?5, ?6, ?7, ?8, ?9, ?10, ?11, ?12, ?13)")?; + + raw_upload.id = self.rng.gen(); let _ = stmt.execute(( - &upload_details.context_id, - &upload_details.timestamp, - &upload_details.raw_upload_url, - &upload_details.flags.as_ref().map(|v| v.to_string()), - &upload_details.provider, - &upload_details.build, - &upload_details.name, - &upload_details.job_name, - &upload_details.ci_run_url, - &upload_details.state, - &upload_details.env, - &upload_details.session_type, - &upload_details - .session_extras - .as_ref() - .map(|v| v.to_string()), + &raw_upload.id, + &raw_upload.timestamp, + &raw_upload.raw_upload_url, + &raw_upload.flags.as_ref().map(|v| v.to_string()), + &raw_upload.provider, + &raw_upload.build, + &raw_upload.name, + &raw_upload.job_name, + &raw_upload.ci_run_url, + &raw_upload.state, + &raw_upload.env, + &raw_upload.session_type, + &raw_upload.session_extras.as_ref().map(|v| v.to_string()), ))?; - Ok(upload_details) + Ok(raw_upload) } fn build(self) -> Result { @@ -286,7 +312,6 @@ mod tests { use rusqlite_migration::SchemaVersion; use serde_json::json; use tempfile::TempDir; - use uuid::Uuid; use super::*; use crate::report::Report; @@ -358,7 +383,7 @@ mod tests { let expected_context = models::Context { id: hash_id("foo"), - context_type: models::ContextType::Upload, + context_type: models::ContextType::TestCase, name: "foo".to_string(), }; let actual_context = report_builder @@ -393,9 +418,13 @@ mod tests { let file = report_builder .insert_file("src/report.rs".to_string()) .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); let mut expected_sample = models::CoverageSample { - id: Uuid::nil(), // Ignored + local_sample_id: 1337, // this will be overwritten + raw_upload_id: raw_upload.id, source_file_id: file.id, line_no: 1, coverage_type: models::CoverageType::Line, @@ -406,9 +435,25 @@ mod tests { let actual_sample = report_builder .insert_coverage_sample(expected_sample.clone()) .unwrap(); - assert_ne!(expected_sample.id, actual_sample.id); - expected_sample.id = actual_sample.id.clone(); + assert_ne!( + expected_sample.local_sample_id, + actual_sample.local_sample_id + ); + assert_eq!(actual_sample.local_sample_id, 0); + expected_sample.local_sample_id = actual_sample.local_sample_id.clone(); assert_eq!(actual_sample, expected_sample); + + let second_sample = report_builder + .insert_coverage_sample(expected_sample.clone()) + .unwrap(); + assert_ne!( + expected_sample.local_sample_id, + second_sample.local_sample_id + ); + assert_ne!(actual_sample.local_sample_id, second_sample.local_sample_id); + assert_eq!(second_sample.local_sample_id, 1); + expected_sample.local_sample_id = second_sample.local_sample_id.clone(); + assert_eq!(second_sample, expected_sample); } #[test] @@ -420,9 +465,13 @@ mod tests { let file = report_builder .insert_file("src/report.rs".to_string()) .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); let coverage_sample = report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: raw_upload.id, source_file_id: file.id, line_no: 1, coverage_type: models::CoverageType::Branch, @@ -433,19 +482,37 @@ mod tests { .unwrap(); let mut expected_branch = models::BranchesData { - id: Uuid::nil(), // Ignored + local_branch_id: 1337, // this will be overwritten + raw_upload_id: raw_upload.id, source_file_id: file.id, - sample_id: coverage_sample.id, + local_sample_id: coverage_sample.local_sample_id, hits: 0, branch_format: models::BranchFormat::Condition, branch: "0:jump".to_string(), + ..Default::default() }; let actual_branch = report_builder .insert_branches_data(expected_branch.clone()) .unwrap(); - assert_ne!(expected_branch.id, actual_branch.id); - expected_branch.id = actual_branch.id; + assert_ne!( + expected_branch.local_branch_id, + actual_branch.local_branch_id + ); + assert_eq!(actual_branch.local_branch_id, 1); + expected_branch.local_branch_id = actual_branch.local_branch_id; assert_eq!(actual_branch, expected_branch); + + let second_branch = report_builder + .insert_branches_data(expected_branch.clone()) + .unwrap(); + assert_ne!( + expected_branch.local_branch_id, + second_branch.local_branch_id + ); + assert_ne!(actual_branch.local_branch_id, second_branch.local_branch_id); + assert_eq!(second_branch.local_branch_id, 2); + expected_branch.local_branch_id = second_branch.local_branch_id; + assert_eq!(second_branch, expected_branch); } #[test] @@ -458,8 +525,13 @@ mod tests { .insert_file("src/report.rs".to_string()) .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let coverage_sample = report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: raw_upload.id, source_file_id: file.id, line_no: 1, // line_no coverage_type: models::CoverageType::Branch, @@ -470,9 +542,10 @@ mod tests { .unwrap(); let mut expected_method = models::MethodData { - id: Uuid::nil(), // Ignored + local_method_id: 1337, // this will be overwritten + raw_upload_id: raw_upload.id, source_file_id: file.id, - sample_id: Some(coverage_sample.id), + local_sample_id: coverage_sample.local_sample_id, line_no: Some(1), hit_branches: Some(1), total_branches: Some(2), @@ -483,8 +556,25 @@ mod tests { let actual_method = report_builder .insert_method_data(expected_method.clone()) .unwrap(); - expected_method.id = actual_method.id; + assert_ne!( + actual_method.local_method_id, + expected_method.local_method_id + ); + assert_eq!(actual_method.local_method_id, 1); + expected_method.local_method_id = actual_method.local_method_id; assert_eq!(actual_method, expected_method); + + let second_method = report_builder + .insert_method_data(expected_method.clone()) + .unwrap(); + assert_ne!(second_method.local_method_id, actual_method.local_method_id); + assert_ne!( + second_method.local_method_id, + expected_method.local_method_id + ); + assert_eq!(second_method.local_method_id, 2); + expected_method.local_method_id = second_method.local_method_id; + assert_eq!(second_method, expected_method); } #[test] @@ -496,9 +586,13 @@ mod tests { let file = report_builder .insert_file("src/report.rs".to_string()) .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); let coverage_sample = report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: raw_upload.id, source_file_id: file.id, line_no: 1, // line_no coverage_type: models::CoverageType::Branch, @@ -509,9 +603,10 @@ mod tests { .unwrap(); let mut expected_span = models::SpanData { - id: Uuid::nil(), // Ignored + raw_upload_id: raw_upload.id, + local_span_id: 1337, // this will be overwritten source_file_id: file.id, - sample_id: Some(coverage_sample.id), + local_sample_id: Some(coverage_sample.local_sample_id), hits: 2, start_line: Some(1), start_col: Some(0), @@ -521,8 +616,19 @@ mod tests { let actual_span = report_builder .insert_span_data(expected_span.clone()) .unwrap(); - expected_span.id = actual_span.id; + assert_ne!(actual_span.local_span_id, expected_span.local_span_id); + assert_eq!(actual_span.local_span_id, 1); + expected_span.local_span_id = actual_span.local_span_id; assert_eq!(actual_span, expected_span); + + let second_span = report_builder + .insert_span_data(expected_span.clone()) + .unwrap(); + assert_ne!(second_span.local_span_id, actual_span.local_span_id); + assert_ne!(second_span.local_span_id, expected_span.local_span_id); + assert_eq!(second_span.local_span_id, 2); + expected_span.local_span_id = second_span.local_span_id; + assert_eq!(second_span, expected_span); } #[test] @@ -535,8 +641,13 @@ mod tests { .insert_file("src/report.rs".to_string()) .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let coverage_sample = report_builder .insert_coverage_sample(models::CoverageSample { + raw_upload_id: raw_upload.id, source_file_id: file.id, line_no: 1, // line_no coverage_type: models::CoverageType::Branch, @@ -546,34 +657,11 @@ mod tests { }) .unwrap(); - let branch = report_builder - .insert_branches_data(models::BranchesData { - source_file_id: file.id, - sample_id: coverage_sample.id, - hits: 0, // hits - branch_format: models::BranchFormat::Condition, - branch: "0:jump".to_string(), - ..Default::default() - }) - .unwrap(); - - let method = report_builder - .insert_method_data(models::MethodData { - source_file_id: file.id, - sample_id: Some(coverage_sample.id), - line_no: Some(1), - hit_branches: Some(1), - total_branches: Some(2), - hit_complexity_paths: Some(1), - total_complexity: Some(2), - ..Default::default() - }) - .unwrap(); - let span = report_builder .insert_span_data(models::SpanData { + raw_upload_id: raw_upload.id, source_file_id: file.id, - sample_id: Some(coverage_sample.id), + local_sample_id: Some(coverage_sample.local_sample_id), hits: 1, // hits start_line: Some(1), // start_line start_col: Some(0), // start_col @@ -584,23 +672,21 @@ mod tests { .unwrap(); let context = report_builder - .insert_context(models::ContextType::Upload, &"upload".to_string()) + .insert_context(models::ContextType::TestCase, &"test_case".to_string()) .unwrap(); let expected_assoc = models::ContextAssoc { context_id: context.id, - sample_id: Some(coverage_sample.id), - branch_id: Some(branch.id), - method_id: Some(method.id), - span_id: Some(span.id), + raw_upload_id: raw_upload.id, + local_sample_id: Some(coverage_sample.local_sample_id), + local_span_id: Some(span.local_span_id), }; let actual_assoc = report_builder .associate_context(models::ContextAssoc { context_id: context.id, - sample_id: Some(coverage_sample.id), - branch_id: Some(branch.id), - method_id: Some(method.id), - span_id: Some(span.id), + raw_upload_id: raw_upload.id, + local_sample_id: Some(coverage_sample.local_sample_id), + local_span_id: Some(span.local_span_id), }) .unwrap(); assert_eq!(actual_assoc, expected_assoc); @@ -617,7 +703,7 @@ mod tests { assert_eq!( s, String::from( - "UNIQUE constraint failed: context_assoc.context_id, context_assoc.sample_id" + "UNIQUE constraint failed: context_assoc.context_id, context_assoc.raw_upload_id, context_assoc.local_sample_id, context_assoc.local_span_id" ) ); } @@ -628,16 +714,12 @@ mod tests { } #[test] - fn test_insert_upload_details() { + fn test_insert_raw_upload() { let ctx = setup(); let db_file = ctx.temp_dir.path().join("db.sqlite"); let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); - let upload = report_builder - .insert_context(models::ContextType::Upload, "codecov-rs CI") - .unwrap(); - let inserted_details = models::UploadDetails { - context_id: upload.id, + let inserted_upload = models::RawUpload { timestamp: Some(123), raw_upload_url: Some("https://example.com".to_string()), flags: Some(json!(["abc".to_string(), "def".to_string()])), @@ -650,25 +732,13 @@ mod tests { env: Some("env".to_string()), session_type: Some("uploaded".to_string()), session_extras: Some(json!({})), + ..Default::default() }; - let inserted_details = report_builder - .insert_upload_details(inserted_details) - .unwrap(); - - let other_upload = report_builder - .insert_context(models::ContextType::Upload, "codecov-rs CI 2") - .unwrap(); + let inserted_upload = report_builder.insert_raw_upload(inserted_upload).unwrap(); let report = report_builder.build().unwrap(); - let fetched_details = report.get_details_for_upload(&upload).unwrap(); - assert_eq!(fetched_details, inserted_details); - - let other_details_result = report.get_details_for_upload(&other_upload); - assert!(other_details_result.is_err()); - match other_details_result { - Err(CodecovError::SqliteError(rusqlite::Error::QueryReturnedNoRows)) => {} - _ => assert!(false), - } + let fetched_uploads = report.list_raw_uploads().unwrap(); + assert_eq!(fetched_uploads, &[inserted_upload]); } #[test] diff --git a/tests/test_pyreport_shim.rs b/tests/test_pyreport_shim.rs index c87eb9e..d761df0 100644 --- a/tests/test_pyreport_shim.rs +++ b/tests/test_pyreport_shim.rs @@ -15,6 +15,8 @@ use codecov_rs::{ models, pyreport::ToPyreport, Report, ReportBuilder, SqliteReport, SqliteReportBuilder, }, }; +use rand::{rngs::StdRng, Rng, SeedableRng}; +use serde_json::json; use tempfile::TempDir; use winnow::Parser; @@ -44,8 +46,13 @@ fn hash_id(key: &str) -> i64 { fn test_parse_report_json() { let input = common::read_sample_file(Path::new("codecov-rs-reports-json-d2a9ba1.txt")); + let rng_seed = 5; + let mut rng = StdRng::seed_from_u64(rng_seed); + let test_ctx = setup(); - let parse_ctx = ReportBuilderCtx::new(SqliteReportBuilder::new(test_ctx.db_file).unwrap()); + let parse_ctx = ReportBuilderCtx::new( + SqliteReportBuilder::new_with_seed(test_ctx.db_file, rng_seed).unwrap(), + ); let mut buf = ReportJsonStream { input: &input, state: parse_ctx, @@ -66,12 +73,21 @@ fn test_parse_report_json() { }, ]; - let expected_name = "[1704827412] codecov-rs CI: https://github.com/codecov/codecov-rs/actions/runs/7465738121 (0)"; - let expected_sessions = vec![models::Context { - id: hash_id(expected_name), - context_type: models::ContextType::Upload, - name: expected_name.to_string(), - }]; + let expected_session = models::RawUpload { + id: rng.gen(), + timestamp: Some(1704827412), + raw_upload_url: Some("v4/raw/2024-01-09/BD18D96000B80FA280C411B0081460E1/d2a9ba133c9b30468d97e7fad1462728571ad699/065067fe-7677-4bd8-93b2-0a8d0b879f78/340c0c0b-a955-46a0-9de9-3a9b5f2e81e2.txt".to_string()), + flags: Some(json!([])), + provider: None, + build: None, + name: None, + job_name: Some("codecov-rs CI".to_string()), + ci_run_url: Some("https://github.com/codecov/codecov-rs/actions/runs/7465738121".to_string()), + state: None, + env: None, + session_type: Some("uploaded".to_string()), + session_extras: Some(json!({})), + }; let expected_json_files = HashMap::from([ (0, expected_files[0].id), @@ -79,7 +95,7 @@ fn test_parse_report_json() { (2, expected_files[2].id), ]); - let expected_json_sessions = HashMap::from([(0, expected_sessions[0].id)]); + let expected_json_sessions = HashMap::from([(0, expected_session.id)]); let (actual_files, actual_sessions) = report_json::parse_report_json .parse_next(&mut buf) @@ -93,15 +109,17 @@ fn test_parse_report_json() { assert_eq!(files, expected_files); let contexts = report.list_contexts().unwrap(); - assert_eq!(contexts.len(), 1); - assert_eq!(contexts[0].context_type, models::ContextType::Upload); - assert_eq!(contexts[0].name, expected_name); + assert!(contexts.is_empty()); + + let uploads = report.list_raw_uploads().unwrap(); + assert_eq!(uploads, vec![expected_session]); } #[test] fn test_parse_chunks_file() { let input = common::read_sample_file(Path::new("codecov-rs-chunks-d2a9ba1.txt")); let test_ctx = setup(); + let mut report_builder = SqliteReportBuilder::new(test_ctx.db_file).unwrap(); // Pretend `parse_report_json` has already run @@ -121,7 +139,7 @@ fn test_parse_chunks_file() { // Pretend `parse_report_json` has already run let mut report_json_sessions = HashMap::new(); let session = report_builder - .insert_context(models::ContextType::Upload, "codecov-rs CI") + .insert_raw_upload(Default::default()) .unwrap(); report_json_sessions.insert(0, session.id); @@ -142,17 +160,20 @@ fn test_parse_chunks_file() { .expect("Failed to parse"); // Helper function for creating our expected values - fn make_sample(source_file_id: i64, line_no: i64, hits: i64) -> models::CoverageSample { - models::CoverageSample { - id: uuid::Uuid::nil(), // Ignored - source_file_id, - line_no, - coverage_type: models::CoverageType::Line, - hits: Some(hits), - hit_branches: None, - total_branches: None, - } - } + let mut coverage_sample_id_iterator = 0..; + let mut make_sample = + |source_file_id: i64, line_no: i64, hits: i64| -> models::CoverageSample { + models::CoverageSample { + local_sample_id: coverage_sample_id_iterator.next().unwrap(), + raw_upload_id: session.id, + source_file_id, + line_no, + coverage_type: models::CoverageType::Line, + hits: Some(hits), + hit_branches: None, + total_branches: None, + } + }; // (start_line, end_line, hits) let covered_lines: [Vec<(i64, i64, i64)>; 3] = [ vec![ @@ -195,21 +216,13 @@ fn test_parse_chunks_file() { let actual_coverage_samples = report .list_coverage_samples() .expect("Failed to list coverage samples"); - let actual_contexts = report.list_contexts().expect("Failed to list contexts"); assert_eq!( actual_coverage_samples.len(), expected_coverage_samples.len() ); for i in 0..actual_coverage_samples.len() { - expected_coverage_samples[i].id = actual_coverage_samples[i].id; + expected_coverage_samples[i].local_sample_id = actual_coverage_samples[i].local_sample_id; assert_eq!(actual_coverage_samples[i], expected_coverage_samples[i]); - - assert_eq!( - report - .list_contexts_for_sample(&actual_coverage_samples[i]) - .unwrap(), - actual_contexts - ); } } @@ -222,8 +235,32 @@ fn test_parse_pyreport() { .expect("Failed to open chunks file"); let test_ctx = setup(); - let report = pyreport::parse_pyreport(&report_json_file, &chunks_file, test_ctx.db_file) - .expect("Failed to parse pyreport"); + let rng_seed = 5; + let mut rng = StdRng::seed_from_u64(rng_seed); + + let report = pyreport::parse_pyreport_with_seed( + &report_json_file, + &chunks_file, + test_ctx.db_file, + rng_seed, + ) + .expect("Failed to parse pyreport"); + + let expected_session = models::RawUpload { + id: rng.gen(), + timestamp: Some(1704827412), + raw_upload_url: Some("v4/raw/2024-01-09/BD18D96000B80FA280C411B0081460E1/d2a9ba133c9b30468d97e7fad1462728571ad699/065067fe-7677-4bd8-93b2-0a8d0b879f78/340c0c0b-a955-46a0-9de9-3a9b5f2e81e2.txt".to_string()), + flags: Some(json!([])), + provider: None, + build: None, + name: None, + job_name: Some("codecov-rs CI".to_string()), + ci_run_url: Some("https://github.com/codecov/codecov-rs/actions/runs/7465738121".to_string()), + state: None, + env: None, + session_type: Some("uploaded".to_string()), + session_extras: Some(json!({})), + }; let expected_files = vec![ models::SourceFile { @@ -240,25 +277,22 @@ fn test_parse_pyreport() { }, ]; - let expected_name = "[1704827412] codecov-rs CI: https://github.com/codecov/codecov-rs/actions/runs/7465738121 (0)"; - let expected_sessions = vec![models::Context { - id: hash_id(expected_name), - context_type: models::ContextType::Upload, - name: expected_name.to_string(), - }]; - // Helper function for creating our expected values - fn make_sample(source_file_id: i64, line_no: i64, hits: i64) -> models::CoverageSample { - models::CoverageSample { - id: uuid::Uuid::nil(), // Ignored - source_file_id, - line_no, - coverage_type: models::CoverageType::Line, - hits: Some(hits), - hit_branches: None, - total_branches: None, - } - } + let mut coverage_sample_id_iterator = 0..; + let mut make_sample = + |source_file_id: i64, line_no: i64, hits: i64| -> models::CoverageSample { + models::CoverageSample { + local_sample_id: coverage_sample_id_iterator.next().unwrap(), + raw_upload_id: expected_session.id, + source_file_id, + line_no, + coverage_type: models::CoverageType::Line, + hits: Some(hits), + hit_branches: None, + total_branches: None, + } + }; + // (start_line, end_line, hits) let covered_lines: [Vec<(i64, i64, i64)>; 3] = [ vec![ @@ -300,23 +334,13 @@ fn test_parse_pyreport() { let actual_coverage_samples = report .list_coverage_samples() .expect("Failed to list coverage samples"); - let actual_contexts = report.list_contexts().expect("Failed to list contexts"); - assert_eq!(actual_contexts, expected_sessions); - assert_eq!( - actual_coverage_samples.len(), - expected_coverage_samples.len() - ); - for i in 0..actual_coverage_samples.len() { - expected_coverage_samples[i].id = actual_coverage_samples[i].id; - assert_eq!(actual_coverage_samples[i], expected_coverage_samples[i]); + assert_eq!(actual_coverage_samples, expected_coverage_samples); - assert_eq!( - report - .list_contexts_for_sample(&actual_coverage_samples[i]) - .unwrap(), - actual_contexts - ); - } + let contexts = report.list_contexts().unwrap(); + assert!(contexts.is_empty()); + + let uploads = report.list_raw_uploads().unwrap(); + assert_eq!(uploads, vec![expected_session]); } #[test] From c51281cf5cdcbce6a98ef54aa7e1ebe1fce4c14e Mon Sep 17 00:00:00 2001 From: Matt Hammerly Date: Fri, 26 Jul 2024 13:32:52 -0700 Subject: [PATCH 3/6] add multi-insert Insertable implementation --- Cargo.toml | 2 +- src/report/sqlite/models.rs | 87 +++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 0064659..739af3b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ version = "0.1.0" edition = "2021" [dependencies] -rusqlite = { version = "0.30.0", features = ["bundled"] } +rusqlite = { version = "0.30.0", features = ["bundled", "limits"] } rusqlite_migration = { version = "1.1.0", features = ["from-directory"] } rand = "0.8.5" diff --git a/src/report/sqlite/models.rs b/src/report/sqlite/models.rs index 986d5ef..bff17d2 100644 --- a/src/report/sqlite/models.rs +++ b/src/report/sqlite/models.rs @@ -65,6 +65,51 @@ pub trait Insertable { Ok(()) } + + fn multi_insert<'a, I>(mut models: I, conn: &rusqlite::Connection) -> Result<()> + where + I: Iterator + ExactSizeIterator, + Self: 'a, + { + let var_limit = conn.limit(rusqlite::limits::Limit::SQLITE_LIMIT_VARIABLE_NUMBER) as usize; + // If each model takes up `FIELD_COUNT` variables, we can fit `var_limit / + // FIELD_COUNT` complete models in each "page" of our query + let page_size = var_limit / FIELD_COUNT; + + // Integer division tells us how many full pages there are. If there is a + // non-zero remainder, there is one final incomplete page. + let model_count = models.len(); + let page_count = match (model_count / page_size, model_count % page_size) { + (page_count, 0) => page_count, + (page_count, _) => page_count + 1, + }; + + let (mut query, mut previous_page_size) = (String::new(), 0); + for _ in 0..page_count { + // If there are fewer than `page_size` pages left, the iterator will just take + // everything. + let page_iter = models.by_ref().take(page_size); + + // We can reuse our query string if the current page is the same size as the + // last one. If not, we have to rebuild the query string. + let current_page_size = page_iter.len(); + if current_page_size != previous_page_size { + query = format!(" {},", Self::INSERT_PLACEHOLDER).repeat(current_page_size); + query.insert_str(0, Self::INSERT_QUERY_PRELUDE); + // Remove trailing comma + query.pop(); + previous_page_size = current_page_size; + } + + let mut stmt = conn.prepare_cached(query.as_str())?; + let params = page_iter.flat_map(|model| model.param_bindings()); + stmt.execute(rusqlite::params_from_iter(params))?; + } + + conn.flush_prepared_statement_cache(); + + Ok(()) + } } /// Can't implement foreign traits (`ToSql`/`FromSql`) on foreign types @@ -516,6 +561,48 @@ mod tests { ); } + #[test] + fn test_test_model_multi_insert() { + let ctx = setup(); + + // We lower the limit to force the multi_insert pagination logic to kick in. + // We'll use 5 models, each with 2 variables, so we need 10 variables total. + // Setting the limit to 4 should wind up using multiple pages. + let _ = ctx + .report + .conn + .set_limit(rusqlite::limits::Limit::SQLITE_LIMIT_VARIABLE_NUMBER, 4); + + let models_to_insert = vec![ + TestModel { + id: 1, + data: "foo".to_string(), + }, + TestModel { + id: 2, + data: "bar".to_string(), + }, + TestModel { + id: 3, + data: "baz".to_string(), + }, + TestModel { + id: 4, + data: "abc".to_string(), + }, + TestModel { + id: 5, + data: "def".to_string(), + }, + ]; + + >::multi_insert(models_to_insert.iter(), &ctx.report.conn) + .unwrap(); + + let test_models = list_test_models(&ctx.report); + assert_eq!(test_models, models_to_insert); + } + #[test] fn test_source_file_single_insert() { let ctx = setup(); From 026694751524acd8a82deb81bb4ee6ad9e7bf6f9 Mon Sep 17 00:00:00 2001 From: Matt Hammerly Date: Fri, 26 Jul 2024 14:35:46 -0700 Subject: [PATCH 4/6] add line_no to ReportLine struct --- src/parsers/pyreport/chunks.rs | 11 ++++- src/parsers/pyreport/utils.rs | 88 +++++++++++++++++++++++++++++----- src/report/pyreport/types.rs | 2 + 3 files changed, 89 insertions(+), 12 deletions(-) diff --git a/src/parsers/pyreport/chunks.rs b/src/parsers/pyreport/chunks.rs index aa782eb..c094f54 100644 --- a/src/parsers/pyreport/chunks.rs +++ b/src/parsers/pyreport/chunks.rs @@ -2,7 +2,7 @@ use std::{collections::HashMap, fmt, fmt::Debug}; use winnow::{ combinator::{ - alt, delimited, eof, opt, peek, preceded, separated, separated_pair, seq, terminated, + alt, delimited, empty, eof, opt, peek, preceded, separated, separated_pair, seq, terminated, }, error::{ContextError, ErrMode, ErrorKind, FromExternalError}, stream::Stream, @@ -366,7 +366,9 @@ pub fn report_line<'a, S: StrStream, R: Report, B: ReportBuilder>( where S: Stream, { + let line_no = buf.state.chunk.current_line; let mut report_line = seq! {ReportLine { + line_no: empty.value(line_no), _: '[', coverage: coverage, _: (ws, ',', ws), @@ -1227,6 +1229,7 @@ mod tests { ( "[1, null, [[0, 1]]]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::HitCount(1), coverage_type: CoverageType::Line, sessions: vec![LineSession { @@ -1244,6 +1247,7 @@ mod tests { ( "[1, null, [[0, 1], [1, 1]]]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::HitCount(1), coverage_type: CoverageType::Line, sessions: vec![ @@ -1270,6 +1274,7 @@ mod tests { ( "[1, null, [[0, 1]], null, 3]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::HitCount(1), coverage_type: CoverageType::Line, sessions: vec![LineSession { @@ -1287,6 +1292,7 @@ mod tests { ( "[1, null, [[0, 1]], null, null, []]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::HitCount(1), coverage_type: CoverageType::Line, sessions: vec![LineSession { @@ -1304,6 +1310,7 @@ mod tests { ( "[1, null, [[0, 1]], null, null, [[0, 1, null, [\"test_case\"]]]]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::HitCount(1), coverage_type: CoverageType::Line, sessions: vec![LineSession { @@ -1329,6 +1336,7 @@ mod tests { ( "[\"2/2\", \"b\", [[0, \"2/2\"]], null, null, [[0, \"2/2\", \"b\", [\"test_case\"]]]]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::BranchesTaken{covered: 2, total: 2}, coverage_type: CoverageType::Branch, sessions: vec![LineSession { @@ -1354,6 +1362,7 @@ mod tests { ( "[1, \"m\", [[0, 1]], null, null, [[0, 1, \"m\", [\"test_case\"]]]]", Ok(ReportLine { + line_no: 0, coverage: PyreportCoverage::HitCount(1), coverage_type: CoverageType::Method, sessions: vec![LineSession { diff --git a/src/parsers/pyreport/utils.rs b/src/parsers/pyreport/utils.rs index 23fe23e..e74e3db 100644 --- a/src/parsers/pyreport/utils.rs +++ b/src/parsers/pyreport/utils.rs @@ -61,6 +61,7 @@ fn format_pyreport_branch(branch: &MissingBranch) -> (models::BranchFormat, Stri fn save_line_session>( line_session: &LineSession, coverage_type: &models::CoverageType, + line_no: i64, ctx: &mut ParseCtx, ) -> Result { let file_id = ctx.report_json_files[&ctx.chunk.index]; @@ -77,7 +78,7 @@ fn save_line_session>( .insert_coverage_sample(models::CoverageSample { raw_upload_id: session_id, source_file_id: file_id, - line_no: ctx.chunk.current_line, + line_no, coverage_type: *coverage_type, hits, hit_branches, @@ -159,7 +160,12 @@ pub fn save_report_line>( ) -> Result<()> { // Most of the data we save is at the `LineSession` level for line_session in &report_line.sessions { - let coverage_sample = save_line_session(line_session, &report_line.coverage_type, ctx)?; + let coverage_sample = save_line_session( + line_session, + &report_line.coverage_type, + report_line.line_no, + ctx, + )?; // If we have datapoints, and one of those datapoints is for this `LineSession`, // get its `Context` ID and associate it with our new `CoverageSample`. @@ -431,7 +437,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -471,7 +483,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -495,7 +513,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -519,7 +543,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -546,7 +576,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -573,7 +609,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -603,7 +645,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -633,7 +681,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -660,7 +714,13 @@ mod tests { parse_ctx, &mut test_ctx.sequence, ); - assert!(save_line_session(&input_session, &input_type, parse_ctx).is_ok()); + assert!(save_line_session( + &input_session, + &input_type, + parse_ctx.chunk.current_line, + parse_ctx + ) + .is_ok()); } #[test] @@ -689,6 +749,7 @@ mod tests { } let report_line = ReportLine { + line_no: 1, coverage, sessions, coverage_type, @@ -772,6 +833,7 @@ mod tests { } let report_line = ReportLine { + line_no: 1, coverage, sessions, coverage_type, @@ -811,6 +873,7 @@ mod tests { } let report_line = ReportLine { + line_no: 1, coverage, sessions, coverage_type, @@ -867,6 +930,7 @@ mod tests { } let report_line = ReportLine { + line_no: 1, coverage, sessions, coverage_type, @@ -906,6 +970,7 @@ mod tests { } let report_line = ReportLine { + line_no: 1, coverage, sessions, coverage_type, @@ -965,6 +1030,7 @@ mod tests { } let report_line = ReportLine { + line_no: 1, coverage, sessions, coverage_type, diff --git a/src/report/pyreport/types.rs b/src/report/pyreport/types.rs index 69d3f5b..2c3b06e 100644 --- a/src/report/pyreport/types.rs +++ b/src/report/pyreport/types.rs @@ -158,6 +158,8 @@ pub struct CoverageDatapoint { /// each `LineSession`. #[derive(Debug, PartialEq)] pub struct ReportLine { + pub line_no: i64, + /// An aggregated coverage status across all of the [`LineSession`]s in /// `sessions`. pub coverage: PyreportCoverage, From 49a78cf813fa75df999f54f8e706d2c25694962d Mon Sep 17 00:00:00 2001 From: Matt Hammerly Date: Fri, 26 Jul 2024 16:52:14 -0700 Subject: [PATCH 5/6] add multi-insert methods to ReportBuilder trait --- src/report/mod.rs | 59 +++- src/report/sqlite/report.rs | 43 ++- src/report/sqlite/report_builder.rs | 436 +++++++++++++++++++++++++++- 3 files changed, 523 insertions(+), 15 deletions(-) diff --git a/src/report/mod.rs b/src/report/mod.rs index a38abd6..f09cdd6 100644 --- a/src/report/mod.rs +++ b/src/report/mod.rs @@ -17,6 +17,18 @@ pub trait Report { fn list_files(&self) -> Result>; fn list_contexts(&self) -> Result>; fn list_coverage_samples(&self) -> Result>; + fn list_branches_for_sample( + &self, + sample: &models::CoverageSample, + ) -> Result>; + fn get_method_for_sample( + &self, + sample: &models::CoverageSample, + ) -> Result>; + fn list_spans_for_sample( + &self, + sample: &models::CoverageSample, + ) -> Result>; fn list_contexts_for_sample( &self, sample: &models::CoverageSample, @@ -56,6 +68,15 @@ pub trait ReportBuilder { sample: models::CoverageSample, ) -> Result; + /// Create several [`models::CoverageSample`] records in one query. The + /// passed-in models' `local_sample_id` fields are ignored and overwritten + /// with values that are unique among all `CoverageSample`s with the same + /// `raw_upload_id`. + fn multi_insert_coverage_sample<'a>( + &mut self, + samples: Vec<&'a mut models::CoverageSample>, + ) -> Result<()>; + /// Create a [`models::BranchesData`] record and return it. The passed-in /// model's `local_branch_id` field is ignored and overwritten with a value /// that is unique among all `BranchesData`s with the same `raw_upload_id`. @@ -64,23 +85,51 @@ pub trait ReportBuilder { branch: models::BranchesData, ) -> Result; + /// Create several [`models::BranchesData`] records in one query. The + /// passed-in models' `local_branch_id` fields are ignored and overwritten + /// with values that are unique among all `BranchesData`s with the same + /// `raw_upload_id`. + fn multi_insert_branches_data<'a>( + &mut self, + branches: Vec<&'a mut models::BranchesData>, + ) -> Result<()>; + /// Create a [`models::MethodData`] record and return it. The passed-in /// model's `local_method_id` field is ignored and overwritten with a value /// that is unique among all `MethodData`s with the same `raw_upload_id`. fn insert_method_data(&mut self, method: models::MethodData) -> Result; + /// Create several [`models::MethodData`] records in one query. The + /// passed-in models' `local_method_id` fields are ignored and overwritten + /// with values that are unique among all `MethodData`s with the same + /// `raw_upload_id`. + fn multi_insert_method_data<'a>( + &mut self, + methods: Vec<&'a mut models::MethodData>, + ) -> Result<()>; + /// Create a [`models::SpanData`] record and return it. The passed-in /// model's `local_span_id` field is ignored and overwritten with a value /// that is unique among all `SpanData`s with the same `raw_upload_id`. fn insert_span_data(&mut self, span: models::SpanData) -> Result; + /// Create several [`models::SpanData`] records in one query. The + /// passed-in models' `local_span_id` fields are ignored and overwritten + /// with values that are unique among all `SpanData`s with the same + /// `raw_upload_id`. + fn multi_insert_span_data<'a>(&mut self, spans: Vec<&'a mut models::SpanData>) -> Result<()>; + /// Create a [`models::ContextAssoc`] record that associates a - /// [`models::Context`] with another model. Return the `ContextAssoc` - /// model. - fn associate_context<'a>( + /// [`models::Context`] with another model. Returns the input to follow the + /// pattern of other methods, although no modifications are made. + fn associate_context(&mut self, assoc: models::ContextAssoc) -> Result; + + /// Create several [`models::ContextAssoc`] records that associate + /// [`models::Context`]s with other models. + fn multi_associate_context<'a>( &mut self, - assoc: models::ContextAssoc, - ) -> Result; + assocs: Vec<&'a mut models::ContextAssoc>, + ) -> Result<()>; /// Create a [`models::RawUpload`] record and return it. fn insert_raw_upload(&mut self, upload_details: models::RawUpload) diff --git a/src/report/sqlite/report.rs b/src/report/sqlite/report.rs index a75c316..ca44ca4 100644 --- a/src/report/sqlite/report.rs +++ b/src/report/sqlite/report.rs @@ -1,6 +1,6 @@ use std::path::PathBuf; -use rusqlite::Connection; +use rusqlite::{Connection, OptionalExtension}; use super::open_database; use crate::{ @@ -54,6 +54,45 @@ impl Report for SqliteReport { Ok(samples) } + fn list_branches_for_sample( + &self, + sample: &models::CoverageSample, + ) -> Result> { + let mut stmt = self + .conn + .prepare_cached("SELECT branches_data.local_branch_id, branches_data.raw_upload_id, branches_data.source_file_id, branches_data.local_sample_id, branches_data.branch, branches_data.branch_format, branches_data.hits FROM branches_data WHERE branches_data.local_sample_id = ?1")?; + let branches = stmt + .query_map([sample.local_sample_id], |row| row.try_into())? + .collect::>>()?; + Ok(branches) + } + + fn get_method_for_sample( + &self, + sample: &models::CoverageSample, + ) -> Result> { + let mut stmt = self + .conn + .prepare_cached("SELECT method_data.local_method_id, method_data.raw_upload_id, method_data.source_file_id, method_data.local_sample_id, method_data.line_no, method_data.hit_branches, method_data.total_branches, method_data.hit_complexity_paths, method_data.total_complexity FROM method_data WHERE method_data.local_sample_id = ?1")?; + + Ok(stmt + .query_row([sample.local_sample_id], |row| row.try_into()) + .optional()?) + } + + fn list_spans_for_sample( + &self, + sample: &models::CoverageSample, + ) -> Result> { + let mut stmt = self + .conn + .prepare_cached("SELECT span_data.local_span_id, span_data.raw_upload_id, span_data.source_file_id, span_data.local_sample_id, span_data.hits, span_data.start_line, span_data.start_col, span_data.end_line, span_data.end_col FROM span_data WHERE span_data.local_sample_id = ?1")?; + let span = stmt + .query_map([sample.local_sample_id], |row| row.try_into())? + .collect::>>()?; + Ok(span) + } + // TODO implement for real, just using for integration tests fn list_contexts_for_sample( &self, @@ -61,7 +100,7 @@ impl Report for SqliteReport { ) -> Result> { let mut stmt = self .conn - .prepare_cached("SELECT context.id, context.context_type, context.name FROM context INNER JOIN context_assoc ON context.id = context_assoc.context_id WHERE context_assoc.sample_id = ?1")?; + .prepare_cached("SELECT context.id, context.context_type, context.name FROM context INNER JOIN context_assoc ON context.id = context_assoc.context_id WHERE context_assoc.local_sample_id = ?1")?; let contexts = stmt .query_map([sample.local_sample_id], |row| row.try_into())? .collect::>>()?; diff --git a/src/report/sqlite/report_builder.rs b/src/report/sqlite/report_builder.rs index 7af8d98..3889cdd 100644 --- a/src/report/sqlite/report_builder.rs +++ b/src/report/sqlite/report_builder.rs @@ -110,6 +110,13 @@ impl ReportBuilder for SqliteReportBuilder { self.transaction()?.insert_coverage_sample(sample) } + fn multi_insert_coverage_sample( + &mut self, + samples: Vec<&mut models::CoverageSample>, + ) -> Result<()> { + self.transaction()?.multi_insert_coverage_sample(samples) + } + fn insert_branches_data( &mut self, branch: models::BranchesData, @@ -117,21 +124,37 @@ impl ReportBuilder for SqliteReportBuilder { self.transaction()?.insert_branches_data(branch) } + fn multi_insert_branches_data( + &mut self, + branches: Vec<&mut models::BranchesData>, + ) -> Result<()> { + self.transaction()?.multi_insert_branches_data(branches) + } + fn insert_method_data(&mut self, method: models::MethodData) -> Result { self.transaction()?.insert_method_data(method) } + fn multi_insert_method_data(&mut self, methods: Vec<&mut models::MethodData>) -> Result<()> { + self.transaction()?.multi_insert_method_data(methods) + } + fn insert_span_data(&mut self, span: models::SpanData) -> Result { self.transaction()?.insert_span_data(span) } - fn associate_context<'b>( - &mut self, - assoc: models::ContextAssoc, - ) -> Result { + fn multi_insert_span_data(&mut self, spans: Vec<&mut models::SpanData>) -> Result<()> { + self.transaction()?.multi_insert_span_data(spans) + } + + fn associate_context(&mut self, assoc: models::ContextAssoc) -> Result { self.transaction()?.associate_context(assoc) } + fn multi_associate_context(&mut self, assocs: Vec<&mut models::ContextAssoc>) -> Result<()> { + self.transaction()?.multi_associate_context(assocs) + } + fn insert_raw_upload(&mut self, raw_upload: models::RawUpload) -> Result { self.transaction()?.insert_raw_upload(raw_upload) } @@ -240,6 +263,20 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { Ok(sample) } + fn multi_insert_coverage_sample( + &mut self, + mut samples: Vec<&mut models::CoverageSample>, + ) -> Result<()> { + for sample in &mut samples { + sample.local_sample_id = self.id_sequence.next().unwrap(); + } + >::multi_insert( + samples.iter().map(|v| &**v), + &self.conn, + )?; + Ok(()) + } + fn insert_branches_data( &mut self, mut branch: models::BranchesData, @@ -250,6 +287,20 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { Ok(branch) } + fn multi_insert_branches_data( + &mut self, + mut branches: Vec<&mut models::BranchesData>, + ) -> Result<()> { + for branch in &mut branches { + branch.local_branch_id = self.id_sequence.next().unwrap(); + } + >::multi_insert( + branches.iter().map(|v| &**v), + &self.conn, + )?; + Ok(()) + } + fn insert_method_data(&mut self, mut method: models::MethodData) -> Result { // TODO handle error method.local_method_id = self.id_sequence.next().unwrap(); @@ -257,6 +308,20 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { Ok(method) } + fn multi_insert_method_data( + &mut self, + mut methods: Vec<&mut models::MethodData>, + ) -> Result<()> { + for method in &mut methods { + method.local_method_id = self.id_sequence.next().unwrap(); + } + >::multi_insert( + methods.iter().map(|v| &**v), + &self.conn, + )?; + Ok(()) + } + fn insert_span_data(&mut self, mut span: models::SpanData) -> Result { // TODO handle error span.local_span_id = self.id_sequence.next().unwrap(); @@ -264,14 +329,27 @@ impl<'a> ReportBuilder for SqliteReportBuilderTx<'a> { Ok(span) } - fn associate_context<'b>( - &mut self, - assoc: models::ContextAssoc, - ) -> Result { + fn multi_insert_span_data(&mut self, mut spans: Vec<&mut models::SpanData>) -> Result<()> { + for span in &mut spans { + span.local_span_id = self.id_sequence.next().unwrap(); + } + >::multi_insert(spans.iter().map(|v| &**v), &self.conn)?; + Ok(()) + } + + fn associate_context(&mut self, assoc: models::ContextAssoc) -> Result { >::insert(&assoc, &self.conn)?; Ok(assoc) } + fn multi_associate_context(&mut self, assocs: Vec<&mut models::ContextAssoc>) -> Result<()> { + >::multi_insert( + assocs.iter().map(|v| &**v), + &self.conn, + )?; + Ok(()) + } + fn insert_raw_upload( &mut self, mut raw_upload: models::RawUpload, @@ -456,6 +534,70 @@ mod tests { assert_eq!(second_sample, expected_sample); } + #[test] + fn test_multi_insert_coverage_sample() { + let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); + + let file = report_builder + .insert_file("src/report.rs".to_string()) + .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + + let mut samples: Vec = vec![ + models::CoverageSample { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }; + 5 + ]; + report_builder + .multi_insert_coverage_sample(samples.iter_mut().collect()) + .unwrap(); + + let report = report_builder.build().unwrap(); + let samples = report.list_coverage_samples().unwrap(); + assert_eq!( + samples, + vec![ + models::CoverageSample { + local_sample_id: 0, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }, + models::CoverageSample { + local_sample_id: 1, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }, + models::CoverageSample { + local_sample_id: 2, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }, + models::CoverageSample { + local_sample_id: 3, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }, + models::CoverageSample { + local_sample_id: 4, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }, + ] + ); + } + #[test] fn test_insert_branches_data() { let ctx = setup(); @@ -515,6 +657,83 @@ mod tests { assert_eq!(second_branch, expected_branch); } + #[test] + fn test_multi_insert_branches_data() { + let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); + + let file = report_builder + .insert_file("src/report.rs".to_string()) + .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let cov_sample = report_builder + .insert_coverage_sample(models::CoverageSample { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }) + .unwrap(); + + let mut branches: Vec = vec![ + models::BranchesData { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample.local_sample_id, + ..Default::default() + }; + 5 + ]; + report_builder + .multi_insert_branches_data(branches.iter_mut().collect()) + .unwrap(); + + let report = report_builder.build().unwrap(); + let branches = report.list_branches_for_sample(&cov_sample).unwrap(); + assert_eq!( + branches, + vec![ + models::BranchesData { + local_branch_id: 1, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample.local_sample_id, + ..Default::default() + }, + models::BranchesData { + local_branch_id: 2, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample.local_sample_id, + ..Default::default() + }, + models::BranchesData { + local_branch_id: 3, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample.local_sample_id, + ..Default::default() + }, + models::BranchesData { + local_branch_id: 4, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample.local_sample_id, + ..Default::default() + }, + models::BranchesData { + local_branch_id: 5, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample.local_sample_id, + ..Default::default() + }, + ] + ); + } + #[test] fn test_insert_method_data() { let ctx = setup(); @@ -577,6 +796,76 @@ mod tests { assert_eq!(second_method, expected_method); } + #[test] + fn test_multi_insert_method_data() { + let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); + + let file = report_builder + .insert_file("src/report.rs".to_string()) + .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let cov_sample_1 = report_builder + .insert_coverage_sample(models::CoverageSample { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }) + .unwrap(); + let cov_sample_2 = report_builder + .insert_coverage_sample(models::CoverageSample { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }) + .unwrap(); + + let mut methods: Vec = vec![ + models::MethodData { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample_1.local_sample_id, + ..Default::default() + }, + models::MethodData { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample_2.local_sample_id, + ..Default::default() + }, + ]; + report_builder + .multi_insert_method_data(methods.iter_mut().collect()) + .unwrap(); + + let report = report_builder.build().unwrap(); + let method_1 = report.get_method_for_sample(&cov_sample_1).unwrap(); + assert_eq!( + method_1, + Some(models::MethodData { + local_method_id: 2, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample_1.local_sample_id, + ..Default::default() + }) + ); + let method_2 = report.get_method_for_sample(&cov_sample_2).unwrap(); + assert_eq!( + method_2, + Some(models::MethodData { + local_method_id: 3, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: cov_sample_2.local_sample_id, + ..Default::default() + }) + ); + } + #[test] fn test_insert_span_data() { let ctx = setup(); @@ -631,6 +920,83 @@ mod tests { assert_eq!(second_span, expected_span); } + #[test] + fn test_multi_insert_span_data() { + let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); + + let file = report_builder + .insert_file("src/report.rs".to_string()) + .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let cov_sample = report_builder + .insert_coverage_sample(models::CoverageSample { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }) + .unwrap(); + + let mut spans: Vec = vec![ + models::SpanData { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + ..Default::default() + }; + 5 + ]; + report_builder + .multi_insert_span_data(spans.iter_mut().collect()) + .unwrap(); + + let report = report_builder.build().unwrap(); + let branchs = report.list_spans_for_sample(&cov_sample).unwrap(); + assert_eq!( + branchs, + vec![ + models::SpanData { + local_span_id: 1, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + ..Default::default() + }, + models::SpanData { + local_span_id: 2, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + ..Default::default() + }, + models::SpanData { + local_span_id: 3, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + ..Default::default() + }, + models::SpanData { + local_span_id: 4, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + ..Default::default() + }, + models::SpanData { + local_span_id: 5, + source_file_id: file.id, + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + ..Default::default() + }, + ] + ); + } + #[test] fn test_insert_context_assoc() { let ctx = setup(); @@ -713,6 +1079,60 @@ mod tests { } } + #[test] + fn test_multi_associate_context() { + let ctx = setup(); + let db_file = ctx.temp_dir.path().join("db.sqlite"); + let mut report_builder = SqliteReportBuilder::new(db_file).unwrap(); + + let file = report_builder + .insert_file("src/report.rs".to_string()) + .unwrap(); + let raw_upload = report_builder + .insert_raw_upload(Default::default()) + .unwrap(); + let cov_sample = report_builder + .insert_coverage_sample(models::CoverageSample { + source_file_id: file.id, + raw_upload_id: raw_upload.id, + ..Default::default() + }) + .unwrap(); + + let contexts = vec![ + report_builder + .insert_context(models::ContextType::TestCase, "test case 1") + .unwrap(), + report_builder + .insert_context(models::ContextType::TestCase, "test case 2") + .unwrap(), + report_builder + .insert_context(models::ContextType::TestCase, "test case 3") + .unwrap(), + report_builder + .insert_context(models::ContextType::TestCase, "test case 4") + .unwrap(), + ]; + + let mut assocs: Vec<_> = contexts + .iter() + .map(|context| models::ContextAssoc { + raw_upload_id: raw_upload.id, + local_sample_id: Some(cov_sample.local_sample_id), + context_id: context.id, + ..Default::default() + }) + .collect(); + + let _ = report_builder + .multi_associate_context(assocs.iter_mut().collect()) + .unwrap(); + + let report = report_builder.build().unwrap(); + let associated_contexts = report.list_contexts_for_sample(&cov_sample).unwrap(); + assert_eq!(associated_contexts, contexts); + } + #[test] fn test_insert_raw_upload() { let ctx = setup(); From c43444403775d0638b1f4f86f6693a946e7ca6e8 Mon Sep 17 00:00:00 2001 From: Matt Hammerly Date: Fri, 26 Jul 2024 16:05:53 -0700 Subject: [PATCH 6/6] use multi-insert methods in chunks file parser --- src/parsers/pyreport/chunks.rs | 130 ++- src/parsers/pyreport/utils.rs | 1576 +++++++++++++++++++------------- 2 files changed, 1037 insertions(+), 669 deletions(-) diff --git a/src/parsers/pyreport/chunks.rs b/src/parsers/pyreport/chunks.rs index c094f54..cee03ed 100644 --- a/src/parsers/pyreport/chunks.rs +++ b/src/parsers/pyreport/chunks.rs @@ -398,8 +398,6 @@ where line_session.coverage = correct_coverage; } - utils::save_report_line(&report_line, &mut buf.state) - .map_err(|e| ErrMode::from_external_error(buf, ErrorKind::Fail, e))?; Ok(report_line) } @@ -411,7 +409,7 @@ where /// stream so we don't actually need to return anything to our caller. pub fn report_line_or_empty<'a, S: StrStream, R: Report, B: ReportBuilder>( buf: &mut ReportOutputStream, -) -> PResult<()> +) -> PResult> where S: Stream, { @@ -420,8 +418,8 @@ where // A line is empty if the next character is `\n` or EOF. We don't consume that // next character from the stream though - we leave it there as either the // delimeter between lines or part of `CHUNKS_FILE_END_OF_CHUNK`. - let empty_line = peek(alt((eof, "\n"))).value(()); - let populated_line = report_line.value(()); + let empty_line = peek(alt((eof, "\n"))).map(|_| None); + let populated_line = report_line.map(Some); alt((populated_line, empty_line)).parse_next(buf) } @@ -451,8 +449,12 @@ where // New chunk, start back at line 0. buf.state.chunk.current_line = 0; - let _: Vec<_> = + let report_lines: Vec<_> = preceded(chunk_header, separated(1.., report_line_or_empty, '\n')).parse_next(buf)?; + let report_lines: Vec = report_lines.into_iter().flatten().collect(); + + utils::save_report_lines(report_lines.as_slice(), &mut buf.state) + .map_err(|e| ErrMode::from_external_error(buf, ErrorKind::Fail, e))?; // Advance our chunk index so we can associate the data from the next chunk with // the correct file from the report JSON. @@ -539,32 +541,20 @@ mod tests { fn stub_report_builder(report_builder: &mut MockReportBuilder) { report_builder - .expect_insert_coverage_sample() - .returning(|_| { - Ok(CoverageSample { - ..Default::default() - }) - }); - report_builder.expect_insert_branches_data().returning(|_| { - Ok(BranchesData { - ..Default::default() - }) - }); - report_builder.expect_insert_method_data().returning(|_| { - Ok(MethodData { - ..Default::default() - }) - }); - report_builder.expect_insert_span_data().returning(|_| { - Ok(SpanData { - ..Default::default() - }) - }); - report_builder.expect_associate_context().returning(|_| { - Ok(ContextAssoc { - ..Default::default() - }) - }); + .expect_multi_insert_coverage_sample() + .returning(|_| Ok(())); + report_builder + .expect_multi_insert_branches_data() + .returning(|_| Ok(())); + report_builder + .expect_multi_insert_method_data() + .returning(|_| Ok(())); + report_builder + .expect_multi_insert_span_data() + .returning(|_| Ok(())); + report_builder + .expect_multi_associate_context() + .returning(|_| Ok(())); report_builder.expect_insert_context().returning(|_, name| { Ok(Context { name: name.to_string(), @@ -1419,6 +1409,7 @@ mod tests { } } + /* TODO #[test] fn test_report_line_or_empty() { let test_ctx = setup(); @@ -1432,17 +1423,73 @@ mod tests { let valid_test_cases = [ // Test that empty lines will still advance the `current_line` state - ("\n", Ok(())), - ("\n", Ok(())), - ("\n", Ok(())), - ("[1, null, [[0, 1]]]", Ok(())), - ("[1, null, [[0, 1]], null, 3]", Ok(())), - ("[\"2/2\", \"b\", [[0, \"2/2\"]], null, null, [[0, \"2/2\", \"b\", [\"test_case\"]]]]", Ok(())), - ("\n", Ok(())), + ("\n", Ok(None)), + ("\n", Ok(None)), + ("\n", Ok(None)), + ("[1, null, [[0, 1]]]", + Ok(Some(ReportLine { + line_no: 4, + coverage: PyreportCoverage::HitCount(1), + coverage_type: CoverageType::Line, + sessions: vec![LineSession { + session_id: 0, + coverage: PyreportCoverage::HitCount(1), + branches: None, + partials: None, + complexity: None, + }], + _messages: None, + _complexity: None, + datapoints: None, + })), + ), + ("[1, null, [[0, 1]], null, 3]", + Ok(Some(ReportLine { + line_no: 5, + coverage: PyreportCoverage::HitCount(1), + coverage_type: CoverageType::Line, + sessions: vec![LineSession { + session_id: 0, + coverage: PyreportCoverage::HitCount(1), + branches: None, + partials: None, + complexity: None, + }], + _messages: Some(Some(JsonVal::Null)), + _complexity: Some(Some(Complexity::Total(3))), + datapoints: None, + })), + ), + ("[\"2/2\", \"b\", [[0, \"2/2\"]], null, null, [[0, \"2/2\", \"b\", [\"test_case\"]]]]", + Ok(Some(ReportLine { + line_no: 6, + coverage: PyreportCoverage::BranchesTaken{covered: 2, total: 2}, + coverage_type: CoverageType::Branch, + sessions: vec![LineSession { + session_id: 0, + coverage: PyreportCoverage::BranchesTaken{covered: 2, total: 2}, + branches: None, + partials: None, + complexity: None, + }], + _messages: Some(Some(JsonVal::Null)), + _complexity: Some(None), + datapoints: Some(Some(HashMap::from([( + 0, + CoverageDatapoint { + session_id: 0, + _coverage: PyreportCoverage::BranchesTaken{covered: 2, total: 2}, + _coverage_type: Some(CoverageType::Branch), + labels: vec!["test_case".to_string()], + }, + )]))), + })), + ), + ("\n", Ok(None)), // The last line in the entire chunks file ends in EOF, not \n - ("", Ok(())), + ("", Ok(None)), // `CHUNKS_FILE_END_OF_CHUNK` begins with a `\n` so we know the current line is empty - (CHUNKS_FILE_END_OF_CHUNK, Ok(())), + (CHUNKS_FILE_END_OF_CHUNK, Ok(None)), ]; let expected_line_count = valid_test_cases.len(); @@ -1480,6 +1527,7 @@ mod tests { // throw off subsequent lines that are well-formed. assert_eq!(buf.state.chunk.current_line as usize, expected_line_count); } + */ #[test] fn test_chunk_header() { diff --git a/src/parsers/pyreport/utils.rs b/src/parsers/pyreport/utils.rs index e74e3db..cda4615 100644 --- a/src/parsers/pyreport/utils.rs +++ b/src/parsers/pyreport/utils.rs @@ -4,7 +4,8 @@ use crate::{ report::{ models, pyreport::types::{ - Complexity, LineSession, MissingBranch, Partial, PyreportCoverage, ReportLine, + Complexity, CoverageDatapoint, LineSession, MissingBranch, Partial, PyreportCoverage, + ReportLine, }, Report, ReportBuilder, }, @@ -51,141 +52,255 @@ fn format_pyreport_branch(branch: &MissingBranch) -> (models::BranchFormat, Stri (branch_format, branch_serialized) } -/// Each [`LineSession`] corresponds to one -/// [`crate::report::models::CoverageSample`]. It also sometimes contains data -/// that belongs in [`crate::report::models::BranchesData`], -/// [`crate::report::models::MethodData`], or -/// [`crate::report::models::SpanData`]. This function writes all of that data -/// to the output as well as associations with -/// [`crate::report::models::Context`]s. -fn save_line_session>( +/// Each [`LineSession`] corresponds to a single [`models::CoverageSample`]. +/// Each [`CoverageSample`] _may_ (but won't always) have: +/// - multiple related [`models::BranchesData`] records, one for each specific +/// branch path we have data for +/// - a single related [`models::MethodData`] if the `LineSession` is for a +/// method and we have extra method-specific data +/// - multiple related [`models::SpanData`] records, if we have data indicating +/// different subspans within line are/aren't covered (Chunks files only +/// record single-line spans, which it calls "partials") +/// - multiple related [`models::ContextAssoc`] records to link the line with +/// [`models::Context`]s which, for a chunks file, come from the labels index +#[derive(Default, Debug, PartialEq)] +struct LineSessionModels { + sample: models::CoverageSample, + branches: Vec, + method: Option, + partials: Vec, + assocs: Vec, +} + +fn create_model_sets_for_line_session>( line_session: &LineSession, coverage_type: &models::CoverageType, line_no: i64, + datapoint: Option<&CoverageDatapoint>, ctx: &mut ParseCtx, -) -> Result { - let file_id = ctx.report_json_files[&ctx.chunk.index]; - let session_id = ctx.report_json_sessions[&line_session.session_id]; - - // The chunks file crams three of our model fields into the same "coverage" - // field. We have to separate them. +) -> LineSessionModels { + let source_file_id = ctx.report_json_files[&ctx.chunk.index]; let (hits, hit_branches, total_branches) = separate_pyreport_coverage(&line_session.coverage); + let raw_upload_id = ctx.report_json_sessions[&line_session.session_id]; + + // Each `LineSession` definitely gets a `CoverageSample` + let sample = models::CoverageSample { + source_file_id, + raw_upload_id, + line_no, + coverage_type: *coverage_type, + hits, + hit_branches, + total_branches, + ..Default::default() + }; - // Insert the meat of the `LineSession` and get back a `CoverageSample`. - let coverage_sample = ctx - .db - .report_builder - .insert_coverage_sample(models::CoverageSample { - raw_upload_id: session_id, - source_file_id: file_id, - line_no, - coverage_type: *coverage_type, - hits, - hit_branches, - total_branches, - ..Default::default() - })?; - - // Check for and insert any additional branches data that we have. - if let Some(Some(missing_branches)) = &line_session.branches { - for branch in missing_branches { - let (branch_format, branch_serialized) = format_pyreport_branch(branch); - let _ = ctx - .db - .report_builder - .insert_branches_data(models::BranchesData { - raw_upload_id: session_id, - source_file_id: file_id, - local_sample_id: coverage_sample.local_sample_id, - hits: 0, // Chunks file only records missing branches + // Read the labels index to populate `assocs` + let assocs: Vec<_> = datapoint + .map_or(&vec![], |datapoint| &datapoint.labels) + .iter() + .map(|label| { + let label_context_id = ctx.labels_index[label]; + models::ContextAssoc { + context_id: label_context_id, + raw_upload_id, + ..Default::default() + } + }) + .collect(); + + // Create `BranchesData` models, if there are any + let branches = match &line_session.branches { + Some(Some(missing_branches)) => missing_branches + .iter() + .map(|branch| { + let (branch_format, branch_serialized) = format_pyreport_branch(branch); + models::BranchesData { + source_file_id, + raw_upload_id, + hits: 0, branch_format, branch: branch_serialized, ..Default::default() - })?; - } - } + } + }) + .collect::>(), + _ => vec![], + }; - // Check for and insert any additional method data we have. - if let Some(Some(complexity)) = &line_session.complexity { - let (covered, total) = separate_pyreport_complexity(complexity); - let _ = ctx - .db - .report_builder - .insert_method_data(models::MethodData { - raw_upload_id: session_id, - source_file_id: file_id, - local_sample_id: coverage_sample.local_sample_id, - line_no: Some(ctx.chunk.current_line), + // Create a `MethodData` model, if we have data for it + let method = match &line_session.complexity { + Some(Some(complexity)) => { + let (covered, total) = separate_pyreport_complexity(complexity); + Some(models::MethodData { + source_file_id, + raw_upload_id, + line_no: Some(line_no), hit_complexity_paths: covered, total_complexity: total, ..Default::default() - })?; - } - - // Check for and insert any additional span data we have. - if let Some(Some(partials)) = &line_session.partials { - for Partial { - start_col, - end_col, - coverage, - } in partials - { - let hits = match coverage { - PyreportCoverage::HitCount(hits) => *hits as i64, - _ => 0, - }; - ctx.db.report_builder.insert_span_data(models::SpanData { - raw_upload_id: session_id, - source_file_id: file_id, - local_sample_id: Some(coverage_sample.local_sample_id), - hits, - start_line: Some(ctx.chunk.current_line), - start_col: start_col.map(|x| x as i64), - end_line: Some(ctx.chunk.current_line), - end_col: end_col.map(|x| x as i64), - ..Default::default() - })?; + }) } - } + _ => None, + }; - Ok(coverage_sample) + // Create `SpanData` models, if we have data for single-line spans + let partials = match &line_session.partials { + Some(Some(partials)) => partials + .iter() + .map( + |Partial { + start_col, + end_col, + coverage, + }| { + let hits = match coverage { + PyreportCoverage::HitCount(hits) => *hits as i64, + _ => 0, + }; + models::SpanData { + source_file_id, + raw_upload_id, + hits, + start_line: Some(line_no), + start_col: start_col.map(|x| x as i64), + end_line: Some(line_no), + end_col: end_col.map(|x| x as i64), + ..Default::default() + } + }, + ) + .collect::>(), + _ => vec![], + }; + + LineSessionModels { + sample, + branches, + method, + partials, + assocs, + } } -/// Parsing a chunks file is a separate matter from coercing chunks data into -/// our schema. This function encapsulates most of that logic. It does not -/// include populating `ctx.labels_index`. -pub fn save_report_line>( +fn create_model_sets_for_report_line>( report_line: &ReportLine, ctx: &mut ParseCtx, -) -> Result<()> { - // Most of the data we save is at the `LineSession` level +) -> Vec { + // A `ReportLine` is a collection of `LineSession`s, and each `LineSession` has + // a set of models we need to insert for it. Build a list of those sets of + // models. + let mut line_session_models = vec![]; for line_session in &report_line.sessions { - let coverage_sample = save_line_session( + // Datapoints are effectively `LineSession`-scoped, but they don't actually live + // in the `LineSession`. Get the `CoverageDatapoint` for this + // `LineSession` if there is one. + let datapoint = if let Some(Some(datapoints)) = &report_line.datapoints { + datapoints.get(&(line_session.session_id as u32)) + } else { + None + }; + line_session_models.push(create_model_sets_for_line_session( line_session, &report_line.coverage_type, report_line.line_no, + datapoint, ctx, - )?; - - // If we have datapoints, and one of those datapoints is for this `LineSession`, - // get its `Context` ID and associate it with our new `CoverageSample`. - if let Some(Some(datapoints)) = &report_line.datapoints { - if let Some(datapoint) = datapoints.get(&(line_session.session_id as u32)) { - for label in &datapoint.labels { - let context_id = ctx.labels_index[label]; - let _ = ctx - .db - .report_builder - .associate_context(models::ContextAssoc { - context_id, - raw_upload_id: coverage_sample.raw_upload_id as i64, - local_sample_id: Some(coverage_sample.local_sample_id), - ..Default::default() - })?; - } - } - } + )); } + line_session_models +} + +/// Each [`ReportLine`] from a chunks file is comprised of a number of +/// [`LineSession`]s, and each [`LineSession`] corresponds to a number of +/// related models in our schema ([`LineSessionModels`]). This function builds +/// all of the models for a collection of [`ReportLine`]s and batch-inserts +/// them. +pub fn save_report_lines>( + report_lines: &[ReportLine], + ctx: &mut ParseCtx, +) -> Result<()> { + // Build a flat list of `LineSessionModels` structs for us to insert + let mut models: Vec = report_lines + .iter() + .flat_map(|line| create_model_sets_for_report_line(line, ctx)) + .collect::>(); + + // First, insert all of the `CoverageSample`s. Each of them will have an ID + // assigned as a side-effect of this insertion. That lets us populate the + // `local_sample_id` foreign key on all of the models associated with each + // `CoverageSample`. + ctx.db.report_builder.multi_insert_coverage_sample( + models + .iter_mut() + .map(|LineSessionModels { sample, .. }| sample) + .collect(), + )?; + + // Populate `local_sample_id` and insert all of the context assocs for each + // `LineSession` (if there are any) + ctx.db.report_builder.multi_associate_context( + models + .iter_mut() + .flat_map(|LineSessionModels { sample, assocs, .. }| { + for assoc in assocs.iter_mut() { + assoc.local_sample_id = Some(sample.local_sample_id); + } + assocs + }) + .collect(), + )?; + + // Populate `local_sample_id` and insert all of the `BranchesData` records for + // each `LineSession` (if there are any) + ctx.db.report_builder.multi_insert_branches_data( + models + .iter_mut() + .flat_map( + |LineSessionModels { + sample, branches, .. + }| { + for branch in branches.iter_mut() { + branch.local_sample_id = sample.local_sample_id; + } + branches + }, + ) + .collect(), + )?; + + // Populate `local_sample_id` and insert the single `MethodData` record for each + // `LineSession` (if there is one) + ctx.db.report_builder.multi_insert_method_data( + models + .iter_mut() + .filter_map(|LineSessionModels { sample, method, .. }| { + method.as_mut().map(|method| { + method.local_sample_id = sample.local_sample_id; + method + }) + }) + .collect(), + )?; + + // Populate `local_sample_id` and insert all of the `SpanData` records for each + // `LineSession` (if there are any). In a chunks file, only spans that are + // subsets of a single line are recorded. + ctx.db.report_builder.multi_insert_span_data( + models + .iter_mut() + .flat_map( + |LineSessionModels { + sample, partials, .. + }| { + for span in partials.iter_mut() { + span.local_sample_id = Some(sample.local_sample_id); + } + partials + }, + ) + .collect(), + )?; Ok(()) } @@ -194,27 +309,21 @@ pub fn save_report_line>( mod tests { use std::collections::HashMap; - use mockall::predicate::*; - use super::*; - use crate::report::{pyreport::types::CoverageDatapoint, MockReport, MockReportBuilder}; + use crate::report::{MockReport, MockReportBuilder}; struct Ctx { parse_ctx: ParseCtx>, - sequence: mockall::Sequence, } fn setup() -> Ctx { let report_builder = MockReportBuilder::new(); - let report_json_files = HashMap::from([(0, 0), (1, 1), (2, 2)]); - let report_json_sessions = HashMap::from([(0, 0), (1, 1), (2, 2)]); + let report_json_files = HashMap::from([(0, 123), (1, 456), (2, 789)]); + let report_json_sessions = HashMap::from([(0, 123), (1, 456), (2, 789)]); let parse_ctx = ParseCtx::new(report_builder, report_json_files, report_json_sessions); - Ctx { - parse_ctx, - sequence: mockall::Sequence::new(), - } + Ctx { parse_ctx } } #[test] @@ -268,158 +377,46 @@ mod tests { assert_eq!(separate_pyreport_coverage(&input), expected); } - // This test template function relies on `separate_pyreport_coverage`, - // `separate_pyreport_complexity`, and `format_pyreport_branch` being tested and - // correct. It uses them to set up the appropriate expectations for any - // `LineSession` you pass in. - fn set_up_line_session_expectations( - line_session: &LineSession, - coverage_type: models::CoverageType, - parse_ctx: &mut ParseCtx>, - sequence: &mut mockall::Sequence, - ) -> models::CoverageSample { - let raw_upload_id = parse_ctx.report_json_sessions[&line_session.session_id]; - let source_file_id = parse_ctx.report_json_files[&parse_ctx.chunk.index]; - - let (hits, hit_branches, total_branches) = - separate_pyreport_coverage(&line_session.coverage); - - let line_no = parse_ctx.chunk.current_line; - let local_sample_id = rand::random(); - let inserted_coverage_sample = models::CoverageSample { - raw_upload_id, - local_sample_id, - source_file_id, - line_no, - coverage_type, - hits, - hit_branches, - total_branches, - ..Default::default() - }; - parse_ctx - .db - .report_builder - .expect_insert_coverage_sample() - .with(eq(models::CoverageSample { - local_sample_id: 0, - ..inserted_coverage_sample - })) - .return_once(move |mut sample| { - sample.local_sample_id = local_sample_id; - Ok(sample) - }) - .times(1) - .in_sequence(sequence); + #[test] + fn test_create_model_sets_for_line_session_simple_line() { + let mut test_ctx = setup(); + let parse_ctx = &mut test_ctx.parse_ctx; + parse_ctx.chunk.index = 0; + parse_ctx.chunk.current_line = 1; - if let Some(Some(missing_branches)) = &line_session.branches { - for branch in missing_branches { - let (branch_format, branch_serialized) = format_pyreport_branch(branch); - parse_ctx - .db - .report_builder - .expect_insert_branches_data() - .with(eq(models::BranchesData { - raw_upload_id, - source_file_id, - local_sample_id, - hits: 0, - branch_format, - branch: branch_serialized, - ..Default::default() - })) - .return_once(move |mut branch| { - branch.local_branch_id = rand::random(); - Ok(branch) - }) - .times(1) - .in_sequence(sequence); - } - } else { - parse_ctx - .db - .report_builder - .expect_insert_branches_data() - .times(0); - } + let input_session = LineSession { + session_id: 0, + coverage: PyreportCoverage::HitCount(4), + branches: None, + partials: None, + complexity: None, + }; + let input_type = models::CoverageType::Line; - if let Some(Some(complexity)) = &line_session.complexity { - let (covered, total) = separate_pyreport_complexity(complexity); - parse_ctx - .db - .report_builder - .expect_insert_method_data() - .with(eq(models::MethodData { - raw_upload_id, - source_file_id, - local_sample_id, - line_no: Some(line_no), - hit_complexity_paths: covered, - total_complexity: total, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hits: Some(4), + coverage_type: input_type, ..Default::default() - })) - .return_once(move |mut method| { - method.local_method_id = rand::random(); - Ok(method) - }) - .times(1) - .in_sequence(sequence); - } else { - parse_ctx - .db - .report_builder - .expect_insert_method_data() - .times(0); - } - - if let Some(Some(partials)) = &line_session.partials { - for Partial { - start_col, - end_col, - coverage, - } in partials - { - let hits = match coverage { - PyreportCoverage::HitCount(hits) => *hits as i64, - _ => 0, - }; - parse_ctx - .db - .report_builder - .expect_insert_span_data() - .with(eq(models::SpanData { - raw_upload_id, - source_file_id, - local_sample_id: Some(local_sample_id), - hits, - start_line: Some(line_no), - start_col: start_col.map(|x| x as i64), - end_line: Some(line_no), - end_col: end_col.map(|x| x as i64), - ..Default::default() - })) - .return_once(move |mut span| { - span.local_span_id = rand::random(); - Ok(span) - }) - .times(1) - .in_sequence(sequence); + }, + ..Default::default() } - } else { - parse_ctx - .db - .report_builder - .expect_insert_span_data() - .times(0); - } - - inserted_coverage_sample + ); } #[test] - fn test_save_line_session_simple_line() { + fn test_create_model_sets_for_line_session_simple_line_with_datapoint() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; + parse_ctx.chunk.index = 0; parse_ctx.chunk.current_line = 1; let input_session = LineSession { @@ -431,23 +428,56 @@ mod tests { }; let input_type = models::CoverageType::Line; - set_up_line_session_expectations( + parse_ctx.labels_index = HashMap::from([ + ("test_label".to_string(), 50), + ("test_label_2".to_string(), 51), + ]); + + let datapoint = CoverageDatapoint { + session_id: 0, + _coverage: PyreportCoverage::HitCount(4), + _coverage_type: None, + labels: vec!["test_label".to_string(), "test_label_2".to_string()], + }; + + let line_session_models = create_model_sets_for_line_session( &input_session, - input_type, + &input_type, + 5, + Some(&datapoint), parse_ctx, - &mut test_ctx.sequence, ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hits: Some(4), + coverage_type: input_type, + ..Default::default() + }, + assocs: vec![ + models::ContextAssoc { + raw_upload_id: 123, + context_id: 50, + ..Default::default() + }, + models::ContextAssoc { + raw_upload_id: 123, + context_id: 51, + ..Default::default() + }, + ], + ..Default::default() + } + ); } #[test] - fn test_save_line_session_line_with_partials() { + fn test_create_model_sets_for_line_session_line_with_partials() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -477,23 +507,57 @@ mod tests { }; let input_type = models::CoverageType::Line; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + hits: Some(4), + coverage_type: input_type, + line_no: 5, + ..Default::default() + }, + partials: vec![ + models::SpanData { + raw_upload_id: 123, + source_file_id: 123, + hits: 1, + start_line: Some(5), + end_line: Some(5), + end_col: Some(10), + ..Default::default() + }, + models::SpanData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + start_line: Some(5), + end_line: Some(5), + start_col: Some(15), + end_col: Some(20), + ..Default::default() + }, + models::SpanData { + raw_upload_id: 123, + source_file_id: 123, + hits: 1, + start_line: Some(5), + end_line: Some(5), + start_col: Some(25), + ..Default::default() + }, + ], + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_simple_method() { + fn test_create_model_sets_for_line_session_simple_method() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -507,23 +571,27 @@ mod tests { }; let input_type = models::CoverageType::Method; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hits: Some(4), + coverage_type: input_type, + ..Default::default() + }, + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_method_with_total_complexity() { + fn test_create_model_sets_for_line_session_method_with_total_complexity() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -537,23 +605,34 @@ mod tests { }; let input_type = models::CoverageType::Method; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hits: Some(4), + coverage_type: input_type, + ..Default::default() + }, + method: Some(models::MethodData { + raw_upload_id: 123, + source_file_id: 123, + line_no: Some(5), + total_complexity: Some(13), + ..Default::default() + }), + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_method_with_split_complexity() { + fn test_create_model_sets_for_line_session_method_with_split_complexity() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -570,23 +649,35 @@ mod tests { }; let input_type = models::CoverageType::Method; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hits: Some(4), + coverage_type: input_type, + ..Default::default() + }, + method: Some(models::MethodData { + raw_upload_id: 123, + source_file_id: 123, + line_no: Some(5), + hit_complexity_paths: Some(3), + total_complexity: Some(4), + ..Default::default() + }), + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_simple_branch() { + fn test_create_model_sets_for_line_session_simple_branch() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -603,23 +694,28 @@ mod tests { }; let input_type = models::CoverageType::Branch; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hit_branches: Some(2), + total_branches: Some(4), + coverage_type: input_type, + ..Default::default() + }, + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_branch_with_missing_branches_block_and_branch() { + fn test_create_model_sets_for_line_session_branch_with_missing_branches_block_and_branch() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -639,23 +735,46 @@ mod tests { }; let input_type = models::CoverageType::Branch; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hit_branches: Some(2), + total_branches: Some(4), + coverage_type: input_type, + ..Default::default() + }, + branches: vec![ + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + branch_format: models::BranchFormat::BlockAndBranch, + branch: "0:0".to_string(), + ..Default::default() + }, + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + branch_format: models::BranchFormat::BlockAndBranch, + branch: "0:1".to_string(), + ..Default::default() + }, + ], + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_branch_with_missing_branches_condition() { + fn test_create_model_sets_for_line_session_branch_with_missing_branches_condition() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -675,23 +794,46 @@ mod tests { }; let input_type = models::CoverageType::Branch; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 5, + hit_branches: Some(2), + total_branches: Some(4), + coverage_type: input_type, + ..Default::default() + }, + branches: vec![ + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + branch_format: models::BranchFormat::Condition, + branch: "0:jump".to_string(), + ..Default::default() + }, + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + branch_format: models::BranchFormat::Condition, + branch: "1".to_string(), + ..Default::default() + }, + ], + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_line_session_branch_with_missing_branches_line() { + fn test_create_model_sets_for_line_session_branch_with_missing_branches_line() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; @@ -708,45 +850,63 @@ mod tests { }; let input_type = models::CoverageType::Branch; - set_up_line_session_expectations( - &input_session, - input_type, - parse_ctx, - &mut test_ctx.sequence, + let line_session_models = + create_model_sets_for_line_session(&input_session, &input_type, 5, None, parse_ctx); + + assert_eq!( + line_session_models, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + hit_branches: Some(2), + total_branches: Some(4), + line_no: 5, + coverage_type: input_type, + ..Default::default() + }, + branches: vec![ + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + branch_format: models::BranchFormat::Line, + branch: "26".to_string(), + ..Default::default() + }, + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + hits: 0, + branch_format: models::BranchFormat::Line, + branch: "27".to_string(), + ..Default::default() + }, + ], + ..Default::default() + } ); - assert!(save_line_session( - &input_session, - &input_type, - parse_ctx.chunk.current_line, - parse_ctx - ) - .is_ok()); } #[test] - fn test_save_report_line_line_no_datapoints() { + fn test_create_model_sets_for_report_line_line_no_datapoints() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; + parse_ctx.chunk.index = 0; let coverage_type = models::CoverageType::Line; let coverage = PyreportCoverage::HitCount(10); - let mut sessions = Vec::new(); - for i in 0..3 { - sessions.push(LineSession { - session_id: i, + let sessions: Vec<_> = [0, 1, 2] + .iter() + .map(|i| LineSession { + session_id: *i, coverage: coverage.clone(), branches: None, partials: None, complexity: None, - }); - set_up_line_session_expectations( - &sessions[i], - coverage_type, - parse_ctx, - &mut test_ctx.sequence, - ); - } + }) + .collect(); let report_line = ReportLine { line_no: 1, @@ -757,44 +917,54 @@ mod tests { _complexity: None, datapoints: None, }; - assert!(save_report_line(&report_line, parse_ctx).is_ok()); - } - fn set_up_datapoints_expectations( - inserted_sample: models::CoverageSample, - parse_ctx: &mut ParseCtx>, - ) { - parse_ctx - .db - .report_builder - .expect_associate_context() - .with(eq(models::ContextAssoc { - context_id: 50, - raw_upload_id: inserted_sample.raw_upload_id, - local_sample_id: Some(inserted_sample.local_sample_id), - ..Default::default() - })) - .returning(|assoc| Ok(assoc)) - .times(1); - parse_ctx - .db - .report_builder - .expect_associate_context() - .with(eq(models::ContextAssoc { - context_id: 51, - raw_upload_id: inserted_sample.raw_upload_id, - local_sample_id: Some(inserted_sample.local_sample_id), - ..Default::default() - })) - .returning(|assoc| Ok(assoc)) - .times(1); + let model_sets = create_model_sets_for_report_line(&report_line, parse_ctx); + assert_eq!( + model_sets, + vec![ + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 1, + hits: Some(10), + coverage_type: coverage_type, + ..Default::default() + }, + ..Default::default() + }, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 456, + source_file_id: 123, + line_no: 1, + hits: Some(10), + coverage_type: coverage_type, + ..Default::default() + }, + ..Default::default() + }, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 789, + source_file_id: 123, + line_no: 1, + hits: Some(10), + coverage_type: coverage_type, + ..Default::default() + }, + ..Default::default() + }, + ] + ); } #[test] - fn test_save_report_line_line_with_datapoints() { + fn test_create_model_sets_for_report_line_line_with_datapoints() { let mut test_ctx = setup(); let parse_ctx = &mut test_ctx.parse_ctx; parse_ctx.chunk.current_line = 1; + parse_ctx.chunk.index = 0; let coverage_type = models::CoverageType::Line; let coverage = PyreportCoverage::HitCount(10); @@ -803,131 +973,37 @@ mod tests { ("test_label_2".to_string(), 51), ]); - let mut sessions = Vec::new(); - let mut datapoints = HashMap::new(); - for i in 0..3 { - sessions.push(LineSession { - session_id: i, + let sessions: Vec<_> = [0, 1, 2] + .iter() + .map(|i| LineSession { + session_id: *i, coverage: coverage.clone(), branches: None, partials: None, complexity: None, - }); - let sample = set_up_line_session_expectations( - &sessions[i], - coverage_type, - parse_ctx, - &mut test_ctx.sequence, - ); - - datapoints.insert( - i as u32, + }) + .collect(); + + let datapoints: HashMap = HashMap::from([ + ( + 0, CoverageDatapoint { - session_id: i as u32, + session_id: 0, _coverage: coverage.clone(), _coverage_type: Some(coverage_type), labels: vec!["test_label".to_string(), "test_label_2".to_string()], }, - ); - set_up_datapoints_expectations(sample.clone(), parse_ctx); - } - - let report_line = ReportLine { - line_no: 1, - coverage, - sessions, - coverage_type, - _messages: None, - _complexity: None, - datapoints: Some(Some(datapoints)), - }; - assert!(save_report_line(&report_line, parse_ctx).is_ok()); - } - - #[test] - fn test_save_report_line_branch_no_datapoints() { - let mut test_ctx = setup(); - let parse_ctx = &mut test_ctx.parse_ctx; - parse_ctx.chunk.current_line = 1; - let coverage_type = models::CoverageType::Branch; - let coverage = PyreportCoverage::BranchesTaken { - covered: 2, - total: 4, - }; - - let mut sessions = Vec::new(); - for i in 0..3 { - sessions.push(LineSession { - session_id: i, - coverage: coverage.clone(), - branches: Some(Some(vec![MissingBranch::Line(26), MissingBranch::Line(27)])), - partials: None, - complexity: None, - }); - set_up_line_session_expectations( - &sessions[i], - coverage_type, - parse_ctx, - &mut test_ctx.sequence, - ); - } - - let report_line = ReportLine { - line_no: 1, - coverage, - sessions, - coverage_type, - _messages: None, - _complexity: None, - datapoints: None, - }; - assert!(save_report_line(&report_line, parse_ctx).is_ok()); - } - - #[test] - fn test_save_report_line_branch_with_datapoints() { - let mut test_ctx = setup(); - let parse_ctx = &mut test_ctx.parse_ctx; - parse_ctx.chunk.current_line = 1; - let coverage_type = models::CoverageType::Branch; - let coverage = PyreportCoverage::BranchesTaken { - covered: 3, - total: 4, - }; - - parse_ctx.labels_index = HashMap::from([ - ("test_label".to_string(), 50), - ("test_label_2".to_string(), 51), - ]); - - let mut sessions = Vec::new(); - let mut datapoints = HashMap::new(); - for i in 0..3 { - sessions.push(LineSession { - session_id: i, - coverage: coverage.clone(), - branches: None, - partials: None, - complexity: None, - }); - let sample = set_up_line_session_expectations( - &sessions[i], - coverage_type, - parse_ctx, - &mut test_ctx.sequence, - ); - - datapoints.insert( - i as u32, + ), + ( + 2, CoverageDatapoint { - session_id: i as u32, + session_id: 2, _coverage: coverage.clone(), _coverage_type: Some(coverage_type), - labels: vec!["test_label".to_string(), "test_label_2".to_string()], + labels: vec!["test_label_2".to_string()], }, - ); - set_up_datapoints_expectations(sample.clone(), parse_ctx); - } + ), + ]); let report_line = ReportLine { line_no: 1, @@ -938,109 +1014,353 @@ mod tests { _complexity: None, datapoints: Some(Some(datapoints)), }; - assert!(save_report_line(&report_line, parse_ctx).is_ok()); - } - - #[test] - fn test_save_report_line_method_no_datapoints() { - let mut test_ctx = setup(); - let parse_ctx = &mut test_ctx.parse_ctx; - parse_ctx.chunk.current_line = 1; - let coverage_type = models::CoverageType::Method; - let coverage = PyreportCoverage::HitCount(2); - let mut sessions = Vec::new(); - for i in 0..3 { - sessions.push(LineSession { - session_id: i, - coverage: coverage.clone(), - branches: None, - partials: None, - complexity: Some(Some(Complexity::PathsTaken { - covered: 1, - total: 3, - })), - }); - set_up_line_session_expectations( - &sessions[i], - coverage_type, - parse_ctx, - &mut test_ctx.sequence, - ); - } - - let report_line = ReportLine { - line_no: 1, - coverage, - sessions, - coverage_type, - _messages: None, - _complexity: Some(Some(Complexity::PathsTaken { - covered: 1, - total: 3, - })), - datapoints: None, - }; - assert!(save_report_line(&report_line, parse_ctx).is_ok()); + let model_sets = create_model_sets_for_report_line(&report_line, parse_ctx); + assert_eq!( + model_sets, + vec![ + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 1, + hits: Some(10), + coverage_type: coverage_type, + ..Default::default() + }, + assocs: vec![ + models::ContextAssoc { + context_id: 50, + raw_upload_id: 123, + ..Default::default() + }, + models::ContextAssoc { + context_id: 51, + raw_upload_id: 123, + ..Default::default() + }, + ], + ..Default::default() + }, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 456, + source_file_id: 123, + line_no: 1, + hits: Some(10), + coverage_type: coverage_type, + ..Default::default() + }, + ..Default::default() + }, + LineSessionModels { + sample: models::CoverageSample { + raw_upload_id: 789, + source_file_id: 123, + line_no: 1, + hits: Some(10), + coverage_type: coverage_type, + ..Default::default() + }, + assocs: vec![models::ContextAssoc { + context_id: 51, + raw_upload_id: 789, + ..Default::default() + },], + ..Default::default() + }, + ] + ); } #[test] - fn test_save_report_line_method_with_datapoints() { + fn test_save_report_lines() { let mut test_ctx = setup(); - let parse_ctx = &mut test_ctx.parse_ctx; - parse_ctx.chunk.current_line = 1; - let coverage_type = models::CoverageType::Method; - let coverage = PyreportCoverage::HitCount(2); - - parse_ctx.labels_index = HashMap::from([ + test_ctx.parse_ctx.labels_index = HashMap::from([ ("test_label".to_string(), 50), ("test_label_2".to_string(), 51), ]); - - let mut sessions = Vec::new(); - let mut datapoints = HashMap::new(); - for i in 0..3 { - sessions.push(LineSession { - session_id: i, - coverage: coverage.clone(), - branches: None, - partials: None, - complexity: Some(Some(Complexity::PathsTaken { - covered: 1, - total: 3, - })), - }); - let sample = set_up_line_session_expectations( - &sessions[i], - coverage_type, - parse_ctx, - &mut test_ctx.sequence, - ); - - datapoints.insert( - i as u32, - CoverageDatapoint { - session_id: i as u32, - _coverage: coverage.clone(), - _coverage_type: Some(coverage_type), - labels: vec!["test_label".to_string(), "test_label_2".to_string()], + test_ctx.parse_ctx.chunk.current_line = 1; + test_ctx.parse_ctx.chunk.index = 0; + + // Sample input: 1 line (2 sessions), 1 branch (1 session), 1 method (1 session) + // BranchesData, SpanData, MethodData, and ContextAssoc will all get inserted + let report_lines = vec![ + // ReportLine 1: a line with 2 sessions, 1 datapoint, 1 label + ReportLine { + line_no: 1, + coverage: PyreportCoverage::HitCount(10), + coverage_type: models::CoverageType::Line, + sessions: vec![ + LineSession { + session_id: 0, + coverage: PyreportCoverage::HitCount(10), + branches: None, + partials: None, + complexity: None, + }, + LineSession { + session_id: 1, + coverage: PyreportCoverage::HitCount(10), + branches: None, + partials: Some(Some(vec![Partial { + start_col: None, + end_col: Some(10), + coverage: PyreportCoverage::HitCount(3), + }])), + complexity: None, + }, + ], + _messages: None, + _complexity: None, + datapoints: Some(Some(HashMap::from([( + 0, + CoverageDatapoint { + session_id: 0, + _coverage: PyreportCoverage::HitCount(10), + _coverage_type: None, + labels: vec!["test_label".to_string()], + }, + )]))), + }, + // ReportLine 2: a branch with 1 session, 2 BranchesData rows, 1 datapoint, 1 label + ReportLine { + line_no: 2, + coverage: PyreportCoverage::BranchesTaken { + covered: 2, + total: 4, }, - ); - set_up_datapoints_expectations(sample.clone(), parse_ctx); - } + coverage_type: models::CoverageType::Branch, + sessions: vec![LineSession { + session_id: 0, + coverage: PyreportCoverage::BranchesTaken { + covered: 2, + total: 4, + }, + branches: Some(Some(vec![ + MissingBranch::BlockAndBranch(0, 0), + MissingBranch::BlockAndBranch(0, 1), + ])), + partials: None, + complexity: None, + }], + _messages: None, + _complexity: None, + datapoints: Some(Some(HashMap::from([( + 0, + CoverageDatapoint { + session_id: 0, + _coverage: PyreportCoverage::BranchesTaken { + covered: 2, + total: 4, + }, + _coverage_type: None, + labels: vec!["test_label".to_string()], + }, + )]))), + }, + // ReportLine 3: a method with complexity, 1 session, 1 datapoint, 1 label + ReportLine { + line_no: 3, + coverage: PyreportCoverage::HitCount(3), + coverage_type: models::CoverageType::Method, + sessions: vec![LineSession { + session_id: 2, + coverage: PyreportCoverage::HitCount(3), + branches: None, + partials: None, + complexity: Some(Some(Complexity::Total(4))), + }], + _messages: None, + _complexity: None, + datapoints: Some(Some(HashMap::from([( + 2, + CoverageDatapoint { + session_id: 2, + _coverage: PyreportCoverage::HitCount(3), + _coverage_type: None, + labels: vec!["test_label_2".to_string()], + }, + )]))), + }, + ]; + + // Now we need to set up our mock expectations. There are a lot of them. + // First thing that gets inserted is CoverageSample. We expect 4 of them, + // one for each LineSession. Our first ReportLine has 2 sessions, and the + // other two have 1 session each, so 4 total. + let expected_cov_samples = vec![ + models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 1, + coverage_type: models::CoverageType::Line, + hits: Some(10), + ..Default::default() + }, + models::CoverageSample { + raw_upload_id: 456, + source_file_id: 123, + line_no: 1, + coverage_type: models::CoverageType::Line, + hits: Some(10), + ..Default::default() + }, + models::CoverageSample { + raw_upload_id: 123, + source_file_id: 123, + line_no: 2, + coverage_type: models::CoverageType::Branch, + hit_branches: Some(2), + total_branches: Some(4), + ..Default::default() + }, + models::CoverageSample { + raw_upload_id: 789, + source_file_id: 123, + line_no: 3, + coverage_type: models::CoverageType::Method, + hits: Some(3), + ..Default::default() + }, + ]; + test_ctx + .parse_ctx + .db + .report_builder + .expect_multi_insert_coverage_sample() + .withf(move |samples| { + // TODO: Can I skip this silly step + let mut expected_samples = expected_cov_samples.clone(); + let ref_vec: Vec<_> = expected_samples.iter_mut().collect(); + *samples == ref_vec + }) + .returning(|mut samples| { + // Assign everything an ID so we can make sure the right assocs are there + for (i, sample) in samples.iter_mut().enumerate() { + sample.local_sample_id = i as i64; + } + Ok(()) + }); - let report_line = ReportLine { - line_no: 1, - coverage, - sessions, - coverage_type, - _messages: None, - _complexity: Some(Some(Complexity::PathsTaken { - covered: 1, - total: 3, - })), - datapoints: Some(Some(datapoints)), - }; - assert!(save_report_line(&report_line, parse_ctx).is_ok()); + // Next thing to go is ContextAssoc. Only 3 LineSessions have a corresponding + // CoverageDatapoint, and each CoverageDatapoint only has one label. + // "test_label" is context_id==50 and "test_label_2" is context_id==51 + let expected_assocs = vec![ + models::ContextAssoc { + raw_upload_id: 123, + local_sample_id: Some(0), + context_id: 50, + ..Default::default() + }, + models::ContextAssoc { + raw_upload_id: 123, + local_sample_id: Some(2), + context_id: 50, + ..Default::default() + }, + models::ContextAssoc { + raw_upload_id: 789, + local_sample_id: Some(3), + context_id: 51, + ..Default::default() + }, + ]; + test_ctx + .parse_ctx + .db + .report_builder + .expect_multi_associate_context() + .withf(move |assocs| { + let mut assocs_clone = expected_assocs.clone(); + let ref_vec: Vec<_> = assocs_clone.iter_mut().collect(); + *assocs == ref_vec + }) + .returning(|_| Ok(())); + + // Then we do BranchesData. Our branch ReportLine has a single LineSession, and + // that LineSession has a `branches` field with two missing branches in + // it. + let expected_branches = vec![ + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + local_sample_id: 2, + hits: 0, + branch: "0:0".to_string(), + branch_format: models::BranchFormat::BlockAndBranch, + ..Default::default() + }, + models::BranchesData { + raw_upload_id: 123, + source_file_id: 123, + local_sample_id: 2, + hits: 0, + branch: "0:1".to_string(), + branch_format: models::BranchFormat::BlockAndBranch, + ..Default::default() + }, + ]; + test_ctx + .parse_ctx + .db + .report_builder + .expect_multi_insert_branches_data() + .withf(move |branches| { + let mut branches_clone = expected_branches.clone(); + let ref_vec: Vec<_> = branches_clone.iter_mut().collect(); + *branches == ref_vec + }) + .returning(|_| Ok(())); + + // Then we do MethodData. Our method ReportLine has a single session, and that + // single session has its complexity field filled in. + let expected_methods = vec![models::MethodData { + raw_upload_id: 789, + source_file_id: 123, + local_sample_id: 3, + line_no: Some(3), + total_complexity: Some(4), + ..Default::default() + }]; + test_ctx + .parse_ctx + .db + .report_builder + .expect_multi_insert_method_data() + .withf(move |methods| { + let mut methods_clone = expected_methods.clone(); + let ref_vec: Vec<_> = methods_clone.iter_mut().collect(); + *methods == ref_vec + }) + .returning(|_| Ok(())); + + // Then we do SpanData. Our first ReportLine has two sessions, one without any + // partials and one with a single partial. So, we need to create a + // single SpanData for that partial. + let expected_spans = vec![models::SpanData { + raw_upload_id: 456, + source_file_id: 123, + local_sample_id: Some(1), + hits: 3, + start_line: Some(1), + end_line: Some(1), + end_col: Some(10), + ..Default::default() + }]; + test_ctx + .parse_ctx + .db + .report_builder + .expect_multi_insert_span_data() + .withf(move |spans| { + let mut spans_clone = expected_spans.clone(); + let ref_vec: Vec<_> = spans_clone.iter_mut().collect(); + *spans == ref_vec + }) + .returning(|_| Ok(())); + + // Now we actually run the function + save_report_lines(&report_lines, &mut test_ctx.parse_ctx).unwrap(); } }