diff --git a/lib/api_projects/src/projects.rs b/lib/api_projects/src/projects.rs index ab386f0b1a..d61ae6970f 100644 --- a/lib/api_projects/src/projects.rs +++ b/lib/api_projects/src/projects.rs @@ -3,7 +3,7 @@ use bencher_endpoint::{ }; use bencher_json::{ JsonDirection, JsonPagination, JsonProject, JsonProjects, ProjectResourceId, ResourceName, - Search, + Sanitize as _, Search, project::{JsonUpdateProject, Visibility}, }; use bencher_rbac::project::Permission; @@ -277,6 +277,7 @@ pub async fn get_one_inner( /// /// Update a project. /// The user must have `edit` permissions for the project. +/// Setting the `bmf_version` field requires a server admin. #[endpoint { method = PATCH, path = "/v0/projects/{project}", @@ -321,6 +322,15 @@ async fn patch_inner( Permission::Edit, )?; + // Only a server admin can move the project's BMF version. + if json_project.bmf_version().is_some() && !auth_user.is_admin(&context.rbac) { + let mut auth_user = auth_user.clone(); + auth_user.sanitize(); + return Err(forbidden_error(format!( + "Only admins can update the `bmf_version` field for a project. User is not an admin: {auth_user:?}", + ))); + } + // Check project visibility if let Some(visibility) = json_project.visibility() { #[cfg(not(feature = "plus"))] diff --git a/lib/api_projects/tests/bmf_version.rs b/lib/api_projects/tests/bmf_version.rs index 1e26bb06bc..fe94d7ee6d 100644 --- a/lib/api_projects/tests/bmf_version.rs +++ b/lib/api_projects/tests/bmf_version.rs @@ -7,12 +7,17 @@ //! The `bmf_version` key of a report payload, end to end through ingest. //! //! The key is a contract: the Bencher Metric Format version it declares is what -//! the whole payload is, and an absent key is version 0. The `json` node parses -//! with that version's leaf only, and a payload whose parsed version differs from -//! its declared version is refused with a 400 that names both versions. Fold is -//! refused for every v1 payload, the empty payload included. +//! the whole payload is. The `json` node parses with that version's leaf only, and +//! a payload whose parsed version differs from its declared version is refused +//! with a 400 that names both versions. Fold is refused for every v1 payload, the +//! empty payload included. +//! +//! The second section is the project's `bmf_version`: the version a payload that +//! declares none is read as. It starts at 0, only a server admin moves it, and an +//! explicit key wins over it in either direction. use bencher_api_tests::{TestServer, TestUser}; +use bencher_json::{BmfVersion, ProjectSlug}; use http::StatusCode; /// A BMF v0 payload: a benchmark maps straight to its measures. @@ -50,7 +55,7 @@ const EMPTY: &str = "{}"; /// A signed up user with an organization and a project to report into. struct Fixture { - project_slug: String, + project_slug: ProjectSlug, user: TestUser, } @@ -63,7 +68,7 @@ async fn fixture(server: &TestServer, label: &str) -> Fixture { .create_project(&user, &org, &format!("Bmf Project {label}")) .await; Fixture { - project_slug: project.slug.to_string(), + project_slug: project.slug, user, } } @@ -544,3 +549,259 @@ async fn report_fold_is_refused_for_an_empty_v1_payload() { server.close().await; } + +// --- The project's version --- + +/// GET one project as `user`. +async fn get_project( + server: &TestServer, + user: &TestUser, + project: &ProjectSlug, +) -> serde_json::Value { + let resp = server + .client + .get(server.api_url(&format!("/v0/projects/{project}"))) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .send() + .await + .expect("Request failed"); + let status = resp.status(); + assert_eq!(status, StatusCode::OK, "GET project: {status}"); + resp.json().await.expect("Failed to parse the project") +} + +/// PATCH one project with whatever body, and return its status and body. +async fn try_patch( + server: &TestServer, + user: &TestUser, + project: &ProjectSlug, + body: &serde_json::Value, +) -> (StatusCode, String) { + let resp = server + .client + .patch(server.api_url(&format!("/v0/projects/{project}"))) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(body) + .send() + .await + .expect("Request failed"); + let status = resp.status(); + let body = resp.text().await.expect("Failed to read the response"); + (status, body) +} + +/// Move the project's `bmf_version` as `user`, who has to be the server admin. +async fn set_bmf_version(server: &TestServer, fixture: &Fixture, bmf_version: BmfVersion) { + let (status, body) = try_patch( + server, + &fixture.user, + &fixture.project_slug, + &serde_json::json!({ "bmf_version": bmf_version }), + ) + .await; + assert_eq!(status, StatusCode::OK, "PATCH bmf_version: {body}"); +} + +/// A new project is at version 0, in the create response and on a GET alike. +#[tokio::test] +async fn project_bmf_version_defaults_to_0() { + let server = TestServer::new().await; + let user = server.signup("Bmf User", "bmfdefault@example.com").await; + let org = server.create_org(&user, "Bmf Default Org").await; + + let resp = server + .client + .post(server.api_url(&format!("/v0/organizations/{}/projects", org.slug))) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&serde_json::json!({ "name": "Bmf Default Project" })) + .send() + .await + .expect("Request failed"); + assert_eq!(resp.status(), StatusCode::CREATED); + let created: serde_json::Value = resp.json().await.expect("Failed to parse the project"); + assert_eq!(created["bmf_version"], serde_json::json!(0), "{created}"); + + let slug: ProjectSlug = created["slug"] + .as_str() + .expect("the project has a slug") + .parse() + .expect("the slug is valid"); + let project = get_project(&server, &user, &slug).await; + assert_eq!(project["bmf_version"], serde_json::json!(0), "{project}"); + + server.close().await; +} + +/// A payload with no key is read at the project's version. +/// +/// On a project at 1 the absent key, `null`, and an explicit 1 all read a v1 +/// payload the same way, and a v0 payload with no key meets the contract as +/// version 1. +#[tokio::test] +async fn absent_bmf_version_is_the_projects_version() { + let server = TestServer::new().await; + let fixture = fixture(&server, "absentv1").await; + set_bmf_version(&server, &fixture, BmfVersion::V1).await; + let v1 = v1_results(); + let v0 = v0_results(); + + let absent = report( + &server, + &fixture, + Post { + results: vec![&v1], + bmf_version: None, + ..Post::default() + }, + ) + .await; + let parameter_set = absent + .pointer("/results/0/0/parameter/set") + .and_then(serde_json::Value::as_object) + .expect("Report result parameter set"); + assert!(!parameter_set.is_empty(), "{absent}"); + + let null = report( + &server, + &fixture, + Post { + results: vec![&v1], + bmf_version: Some(serde_json::Value::Null), + ..Post::default() + }, + ) + .await; + let one = report( + &server, + &fixture, + Post { + results: vec![&v1], + bmf_version: Some(serde_json::json!(1)), + ..Post::default() + }, + ) + .await; + assert_eq!(absent, one); + assert_eq!(null, one); + + let body = refused( + &server, + &fixture, + Post { + results: vec![&v0], + bmf_version: None, + ..Post::default() + }, + ) + .await; + assert_names_both_versions(&body, 0, 1); + + server.close().await; +} + +/// The project's version is a default, not a ceiling: a declared key is read as +/// declared on any project, in either direction. +#[tokio::test] +async fn explicit_bmf_version_wins_over_the_projects_version() { + let server = TestServer::new().await; + // The first signup is the server admin, so only this fixture can move its project. + let at_one = fixture(&server, "atone").await; + set_bmf_version(&server, &at_one, BmfVersion::V1).await; + let at_zero = fixture(&server, "atzero").await; + let v0 = v0_results(); + let v1 = v1_results(); + + report( + &server, + &at_one, + Post { + results: vec![&v0], + bmf_version: Some(serde_json::json!(0)), + ..Post::default() + }, + ) + .await; + report( + &server, + &at_zero, + Post { + results: vec![&v1], + bmf_version: Some(serde_json::json!(1)), + ..Post::default() + }, + ) + .await; + + server.close().await; +} + +/// Nothing ratchets: the admin moves the version up and back down. +#[tokio::test] +async fn admin_moves_the_project_bmf_version_both_ways() { + let server = TestServer::new().await; + let fixture = fixture(&server, "bothways").await; + + set_bmf_version(&server, &fixture, BmfVersion::V1).await; + let project = get_project(&server, &fixture.user, &fixture.project_slug).await; + assert_eq!(project["bmf_version"], serde_json::json!(1), "{project}"); + + set_bmf_version(&server, &fixture, BmfVersion::V0).await; + let project = get_project(&server, &fixture.user, &fixture.project_slug).await; + assert_eq!(project["bmf_version"], serde_json::json!(0), "{project}"); + + server.close().await; +} + +/// Only a server admin can set the field, and the rest of the patch is unaffected. +/// +/// The second user owns their own organization and project, so they are allowed to +/// edit it. The only thing standing between them and the field is the admin check. +#[tokio::test] +async fn non_admin_cannot_set_the_project_bmf_version() { + let server = TestServer::new().await; + // The first signup is the server admin, so the second one is not. + let _admin = fixture(&server, "owner").await; + let user = server.signup("Other User", "bmfother@example.com").await; + let org = server.create_org(&user, "Bmf Other Org").await; + let project = server + .create_project(&user, &org, "Bmf Other Project") + .await; + + // The refused patch applies nothing, the rename included. + let (status, body) = try_patch( + &server, + &user, + &project.slug, + &serde_json::json!({ "name": "Bmf Renamed Project", "bmf_version": 1 }), + ) + .await; + assert_eq!(status, StatusCode::FORBIDDEN, "{body}"); + assert!(body.contains("bmf_version"), "{body}"); + let unchanged = get_project(&server, &user, &project.slug).await; + assert_eq!(unchanged["name"], serde_json::json!("Bmf Other Project")); + assert_eq!(unchanged["bmf_version"], serde_json::json!(0)); + + // The same patch without the field is the patch it has always been. + let (status, body) = try_patch( + &server, + &user, + &project.slug, + &serde_json::json!({ "name": "Bmf Renamed Project" }), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + let renamed = get_project(&server, &user, &project.slug).await; + assert_eq!(renamed["name"], serde_json::json!("Bmf Renamed Project")); + assert_eq!(renamed["bmf_version"], serde_json::json!(0)); + + server.close().await; +} diff --git a/lib/api_projects/tests/metric_migration.rs b/lib/api_projects/tests/metric_migration.rs index 986fb96a3f..f8acf1b7aa 100644 --- a/lib/api_projects/tests/metric_migration.rs +++ b/lib/api_projects/tests/metric_migration.rs @@ -719,8 +719,13 @@ fn apply_migration(server: &TestServer) { /// It is not the last migration any more, so reverting only the last one would /// revert someone else's. Each layer above it is reverted first, and /// `run_pending_migrations` puts them all back. +/// +/// The project `bmf_version` migration is put back immediately, because the +/// compiled `QueryProject` selects that column on every project lookup. A later +/// layer that adds a column the model reads belongs on this list too. fn revert_migration(conn: &mut DbConnection) { const METRIC_MIGRATION: &str = "20260816120000"; + const REAPPLIED_MIGRATIONS: &[&str] = &["20260826120000"]; conn.batch_execute("PRAGMA foreign_keys = OFF") .expect("Failed to disable foreign keys"); @@ -732,6 +737,19 @@ fn revert_migration(conn: &mut DbConnection) { break; } } + + let migrations = + diesel::migration::MigrationSource::::migrations(&MIGRATIONS) + .expect("Failed to read the migrations"); + for version in REAPPLIED_MIGRATIONS { + let migration = migrations + .iter() + .find(|migration| migration.name().version().to_string() == **version) + .expect("Failed to find the migration to re-apply"); + conn.run_migration(migration.as_ref()) + .expect("Failed to re-apply the migration"); + } + conn.batch_execute("PRAGMA foreign_keys = ON") .expect("Failed to enable foreign keys"); } diff --git a/lib/api_run/tests/run.rs b/lib/api_run/tests/run.rs index e10d80e178..6806665c43 100644 --- a/lib/api_run/tests/run.rs +++ b/lib/api_run/tests/run.rs @@ -10,9 +10,9 @@ use bencher_api_tests::TestServer; #[cfg(feature = "plus")] use bencher_api_tests::oci::compute_digest; +use bencher_json::{BmfVersion, JsonReport, JsonReports}; #[cfg(feature = "plus")] -use bencher_json::{BmfVersion, JsonJob, JsonRunners, JsonSpec, runner::JsonJobs}; -use bencher_json::{JsonReport, JsonReports}; +use bencher_json::{JsonJob, JsonRunners, JsonSpec, runner::JsonJobs}; use http::StatusCode; // POST /v0/run - create a run with authentication @@ -265,6 +265,98 @@ async fn run_post_unknown_bmf_version_is_rejected() { assert_eq!(report_count(&server, &user, project_slug).await, 0); } +/// POST /v0/run - a run with no `bmf_version` is read at the project's version. +/// +/// The user is the server's first signup, which makes them its admin, so they can +/// move the project to version 1. Then v1 results with no key ingest, and v0 +/// results with no key meet the contract as version 1. +#[tokio::test] +async fn run_post_absent_bmf_version_is_the_projects_version() { + let server = TestServer::new().await; + let user = server.signup("Test User", "runbmfabsent@example.com").await; + let org = server.create_org(&user, "Run Bmf Absent Org").await; + let project = server + .create_project(&user, &org, "Run Bmf Absent Project") + .await; + set_project_bmf_version(&server, &user, &project, BmfVersion::V1).await; + + let project_slug: &str = project.slug.as_ref(); + let run = |results: serde_json::Value| { + serde_json::json!({ + "project": project_slug, + "branch": "main", + "testbed": "localhost", + "start_time": "2024-01-01T00:00:00Z", + "end_time": "2024-01-01T00:01:00Z", + "results": [results.to_string()], + }) + }; + + let resp = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&run(bmf_v1_results())) + .send() + .await + .expect("Request failed"); + assert_eq!(resp.status(), StatusCode::CREATED); + let report: JsonReport = resp.json().await.expect("Failed to parse response"); + let results = report.results.expect("Report results"); + let iteration = results.first().expect("Report iteration"); + let result = iteration.first().expect("Report result"); + assert!(!result.parameter.set.is_empty()); + + let resp = server + .client + .post(server.api_url("/v0/run")) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&run(bmf_results())) + .send() + .await + .expect("Request failed"); + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = resp.text().await.expect("Failed to read the response"); + assert!( + body.contains("parsed as BMF version 0") && body.contains("declared version 1"), + "expected the refusal to name both versions: {body}" + ); + assert!( + !body.contains("right adapter"), + "expected no adapter hint on a version refusal: {body}" + ); +} + +/// Move the project's `bmf_version` as `user`, who has to be the server admin. +async fn set_project_bmf_version( + server: &TestServer, + user: &bencher_api_tests::TestUser, + project: &bencher_api_tests::TestProject, + bmf_version: BmfVersion, +) { + let project_slug: &str = project.slug.as_ref(); + let resp = server + .client + .patch(server.api_url(&format!("/v0/projects/{project_slug}"))) + .header( + bencher_json::AUTHORIZATION, + bencher_json::bearer_header(&user.token), + ) + .json(&serde_json::json!({ "bmf_version": bmf_version })) + .send() + .await + .expect("Request failed"); + let status = resp.status(); + let body = resp.text().await.expect("Failed to read the response"); + assert_eq!(status, StatusCode::OK, "PATCH bmf_version: {body}"); +} + /// How many reports the project holds, so a rejection can be shown to create none. async fn report_count( server: &TestServer, @@ -2038,11 +2130,17 @@ async fn run_post_with_job_config_fields() { } } -/// The declared `bmf_version` rides into the job config beside `average` and -/// `fold`, as the payload declared it: `Some` when the key was sent and `None` -/// when it was absent. +/// The resolved `bmf_version` rides into the job config beside `average` and +/// `fold`: the version the payload declared, or the project's when it declared none. +/// +/// The user is the server's first signup, which makes them its admin, so they can +/// put the project at `project_version` before the run. #[cfg(feature = "plus")] -async fn stored_job_bmf_version(bmf_version: Option, label: &str) -> Option { +async fn stored_job_bmf_version( + project_version: BmfVersion, + bmf_version: Option, + label: &str, +) -> Option { let server = TestServer::new().await; let user = server .signup("Job User", &format!("runjob_bmf_{label}@example.com")) @@ -2053,6 +2151,7 @@ async fn stored_job_bmf_version(bmf_version: Option, label: &str) -> Option< let project = server .create_project(&user, &org, &format!("Bmf Job Project {label}")) .await; + set_project_bmf_version(&server, &user, &project, project_version).await; create_fallback_spec(&server, &user).await; @@ -2109,20 +2208,36 @@ async fn stored_job_bmf_version(bmf_version: Option, label: &str) -> Option< #[tokio::test] async fn run_post_with_job_stores_bmf_version() { assert_eq!( - stored_job_bmf_version(Some(1), "declared").await, + stored_job_bmf_version(BmfVersion::V0, Some(1), "declared").await, Some(BmfVersion::V1), - "a job declared at version 1 carries the key" + "a job declared at version 1 carries version 1" ); } -// POST /v0/run with job and no bmf_version stores no key on the job config +// POST /v0/run with job and an explicit bmf_version 0 on a project at 1 stores 0 #[cfg(feature = "plus")] #[tokio::test] -async fn run_post_with_job_stores_no_bmf_version_when_absent() { +async fn run_post_with_job_stores_explicit_bmf_version_0_on_a_project_at_1() { assert_eq!( - stored_job_bmf_version(None, "absent").await, - None, - "a job posted without the key carries none" + stored_job_bmf_version(BmfVersion::V1, Some(0), "declared0").await, + Some(BmfVersion::V0), + "a job declared at version 0 carries version 0 whatever the project says" + ); +} + +// POST /v0/run with job and no bmf_version stores the project's version on the job config +#[cfg(feature = "plus")] +#[tokio::test] +async fn run_post_with_job_stores_the_projects_bmf_version_when_absent() { + assert_eq!( + stored_job_bmf_version(BmfVersion::V0, None, "absent0").await, + Some(BmfVersion::V0), + "a job posted without the key on a project at 0 carries version 0" + ); + assert_eq!( + stored_job_bmf_version(BmfVersion::V1, None, "absent1").await, + Some(BmfVersion::V1), + "a job posted without the key on a project at 1 carries version 1" ); } diff --git a/lib/bencher_adapter/src/adapters/json/mod.rs b/lib/bencher_adapter/src/adapters/json/mod.rs index 048f454820..478426ce57 100644 --- a/lib/bencher_adapter/src/adapters/json/mod.rs +++ b/lib/bencher_adapter/src/adapters/json/mod.rs @@ -10,8 +10,8 @@ use v1::AdapterJsonV1; /// The `json` node of the adapter tree, over the `json_v0` and `json_v1` leaves. /// -/// The payload's declared `bmf_version` picks the one leaf this node parses with, -/// and an absent key is version 0. A payload that leaf does not claim fails the node. +/// The payload's `bmf_version` picks the one leaf this node parses with, and a +/// payload that leaf does not claim fails the node. pub struct AdapterJson; impl Adaptable for AdapterJson { @@ -225,7 +225,7 @@ pub(crate) mod test_json { } } - /// An absent `bmf_version` is version 0, so the two parse to the same bytes. + /// The default settings are version 0, so the two parse to the same bytes. #[test] fn adapter_json_absent_version_is_version_0() { for suffix in JSON_FIXTURES { diff --git a/lib/bencher_adapter/src/lib.rs b/lib/bencher_adapter/src/lib.rs index 2a954a67d9..9700174ae8 100644 --- a/lib/bencher_adapter/src/lib.rs +++ b/lib/bencher_adapter/src/lib.rs @@ -93,7 +93,8 @@ impl Adaptable for Adapter { #[derive(Debug, Clone, Copy, Default)] pub struct Settings { pub average: Option, - /// The BMF version the report payload declared, where an absent key is version 0. + /// The BMF version the report payload is read as: the one it declared, or its + /// project's default. /// /// The `json` node parses with that version's leaf only, and the results array /// refuses any payload whose parsed version differs. diff --git a/lib/bencher_comment/src/lib.rs b/lib/bencher_comment/src/lib.rs index b97830232b..3eb82dfd7b 100644 --- a/lib/bencher_comment/src/lib.rs +++ b/lib/bencher_comment/src/lib.rs @@ -1192,6 +1192,7 @@ mod tests { slug: "my-project".parse().unwrap(), url: None, visibility, + bmf_version: bencher_json::BmfVersion::default(), created: DateTime::TEST, modified: DateTime::TEST, claimed: Some(DateTime::TEST), diff --git a/lib/bencher_json/src/project/mod.rs b/lib/bencher_json/src/project/mod.rs index 86b5499558..1abcf77130 100644 --- a/lib/bencher_json/src/project/mod.rs +++ b/lib/bencher_json/src/project/mod.rs @@ -3,7 +3,7 @@ use std::{ str::FromStr, }; -use bencher_valid::{DateTime, ResourceId, ResourceName, Url}; +use bencher_valid::{BmfVersion, DateTime, ResourceId, ResourceName, Url}; #[cfg(feature = "schema")] use schemars::JsonSchema; use serde::{ @@ -79,6 +79,7 @@ pub struct JsonProject { pub slug: ProjectSlug, pub url: Option, pub visibility: Visibility, + pub bmf_version: BmfVersion, pub created: DateTime, pub modified: DateTime, pub claimed: Option, @@ -124,6 +125,8 @@ pub struct JsonProjectPatch { /// ➕ Bencher Plus: Set the new visibility of the project. /// Moving to a `private` project requires a valid Bencher Plus subscription. pub visibility: Option, + /// Server admin only: Set the default BMF version for the project. + pub bmf_version: Option, } #[derive(Debug, Clone, Serialize)] @@ -133,6 +136,7 @@ pub struct JsonProjectPatchNull { pub slug: Option, pub url: (), pub visibility: Option, + pub bmf_version: Option, } impl<'de> Deserialize<'de> for JsonUpdateProject { @@ -144,7 +148,14 @@ impl<'de> Deserialize<'de> for JsonUpdateProject { const SLUG_FIELD: &str = "slug"; const URL_FIELD: &str = "url"; const VISIBILITY_FIELD: &str = "visibility"; - const FIELDS: &[&str] = &[NAME_FIELD, SLUG_FIELD, URL_FIELD, VISIBILITY_FIELD]; + const BMF_VERSION_FIELD: &str = "bmf_version"; + const FIELDS: &[&str] = &[ + NAME_FIELD, + SLUG_FIELD, + URL_FIELD, + VISIBILITY_FIELD, + BMF_VERSION_FIELD, + ]; #[derive(Deserialize)] #[serde(field_identifier, rename_all = "snake_case")] @@ -153,6 +164,7 @@ impl<'de> Deserialize<'de> for JsonUpdateProject { Slug, Url, Visibility, + BmfVersion, } struct UpdateProjectVisitor; @@ -172,6 +184,7 @@ impl<'de> Deserialize<'de> for JsonUpdateProject { let mut slug = None; let mut url = None; let mut visibility = None; + let mut bmf_version = None; while let Some(key) = map.next_key()? { match key { @@ -199,6 +212,12 @@ impl<'de> Deserialize<'de> for JsonUpdateProject { } visibility = Some(map.next_value()?); }, + Field::BmfVersion => { + if bmf_version.is_some() { + return Err(de::Error::duplicate_field(BMF_VERSION_FIELD)); + } + bmf_version = Some(map.next_value()?); + }, } } @@ -208,18 +227,21 @@ impl<'de> Deserialize<'de> for JsonUpdateProject { slug, url: Some(url), visibility, + bmf_version, }), Some(None) => Self::Value::Null(JsonProjectPatchNull { name, slug, url: (), visibility, + bmf_version, }), None => Self::Value::Patch(JsonProjectPatch { name, slug, url: None, visibility, + bmf_version, }), }) } @@ -236,6 +258,13 @@ impl JsonUpdateProject { Self::Null(patch) => patch.visibility, } } + + pub fn bmf_version(&self) -> Option { + match self { + Self::Patch(patch) => patch.bmf_version, + Self::Null(patch) => patch.bmf_version, + } + } } const PUBLIC_INT: i32 = 0; diff --git a/lib/bencher_json/src/project/report.rs b/lib/bencher_json/src/project/report.rs index ab88e38461..a42c07e309 100644 --- a/lib/bencher_json/src/project/report.rs +++ b/lib/bencher_json/src/project/report.rs @@ -56,7 +56,7 @@ pub struct JsonNewReport { pub results: Vec, /// The Bencher Metric Format (BMF) version this report is written in. /// The accepted versions are 0 or 1. - /// If no version is specified, then version 0 is used. + /// If no version is specified, then the project's `bmf_version` is used. pub bmf_version: Option, /// Settings for how to handle the report. pub settings: Option, diff --git a/lib/bencher_json/src/run.rs b/lib/bencher_json/src/run.rs index 644ce7ddf8..b524124164 100644 --- a/lib/bencher_json/src/run.rs +++ b/lib/bencher_json/src/run.rs @@ -69,7 +69,7 @@ pub struct JsonNewRun { pub results: Vec, /// The Bencher Metric Format (BMF) version these results are written in. /// The accepted versions are 0 or 1. - /// If no version is specified, then version 0 is used. + /// If no version is specified, then the project's `bmf_version` is used. pub bmf_version: Option, /// Settings for how to handle the results. pub settings: Option, diff --git a/lib/bencher_json/src/runner/job.rs b/lib/bencher_json/src/runner/job.rs index b46c35aecc..a76126dcaa 100644 --- a/lib/bencher_json/src/runner/job.rs +++ b/lib/bencher_json/src/runner/job.rs @@ -391,7 +391,7 @@ pub struct JsonJobConfig { /// Fold operation for combining multiple iteration results #[serde(skip_serializing_if = "Option::is_none")] pub fold: Option, - /// The Bencher Metric Format (BMF) version the run declared + /// The Bencher Metric Format (BMF) version the run's results are read as #[serde(skip_serializing_if = "Option::is_none")] pub bmf_version: Option, /// Allow benchmark failure without short-circuiting iterations diff --git a/lib/bencher_plot/decimal.json b/lib/bencher_plot/decimal.json index 69d7b4755a..129d5cecec 100644 --- a/lib/bencher_plot/decimal.json +++ b/lib/bencher_plot/decimal.json @@ -6,6 +6,7 @@ "slug": "the-computer", "url": null, "visibility": "public", + "bmf_version": 0, "created": "2023-07-02T12:53:33Z", "modified": "2023-07-02T12:53:33Z" }, diff --git a/lib/bencher_plot/perf.json b/lib/bencher_plot/perf.json index 28c805b1a0..4bc6aba785 100644 --- a/lib/bencher_plot/perf.json +++ b/lib/bencher_plot/perf.json @@ -6,6 +6,7 @@ "slug": "the-computer", "url": null, "visibility": "public", + "bmf_version": 0, "created": "2023-07-02T12:53:33Z", "modified": "2023-07-02T12:53:33Z" }, diff --git a/lib/bencher_plot/perf_dual_axes.json b/lib/bencher_plot/perf_dual_axes.json index 5cd47966da..d913b66e63 100644 --- a/lib/bencher_plot/perf_dual_axes.json +++ b/lib/bencher_plot/perf_dual_axes.json @@ -6,6 +6,7 @@ "slug": "the-computer", "url": null, "visibility": "public", + "bmf_version": 0, "created": "2023-07-02T12:53:33Z", "modified": "2023-07-02T12:53:33Z" }, diff --git a/lib/bencher_plot/perf_log.json b/lib/bencher_plot/perf_log.json index 63194e60fb..c2774ac7b8 100644 --- a/lib/bencher_plot/perf_log.json +++ b/lib/bencher_plot/perf_log.json @@ -6,6 +6,7 @@ "slug": "the-computer", "url": null, "visibility": "public", + "bmf_version": 0, "created": "2023-07-02T12:53:33Z", "modified": "2023-07-02T12:53:33Z" }, diff --git a/lib/bencher_plot/perf_missing_value.json b/lib/bencher_plot/perf_missing_value.json index 3b0fefc062..eb6e65d758 100644 --- a/lib/bencher_plot/perf_missing_value.json +++ b/lib/bencher_plot/perf_missing_value.json @@ -6,6 +6,7 @@ "slug": "the-computer", "url": null, "visibility": "public", + "bmf_version": 0, "created": "2023-07-02T12:53:33Z", "modified": "2023-07-02T12:53:33Z" }, diff --git a/lib/bencher_plot/perf_variants.json b/lib/bencher_plot/perf_variants.json index d40cc68735..7096d5c7f4 100644 --- a/lib/bencher_plot/perf_variants.json +++ b/lib/bencher_plot/perf_variants.json @@ -6,6 +6,7 @@ "slug": "the-computer", "url": null, "visibility": "public", + "bmf_version": 0, "created": "2023-07-02T12:53:33Z", "modified": "2023-07-02T12:53:33Z" }, diff --git a/lib/bencher_schema/migrations/2026-08-26-120000_project_bmf_version/down.sql b/lib/bencher_schema/migrations/2026-08-26-120000_project_bmf_version/down.sql new file mode 100644 index 0000000000..8834bac278 --- /dev/null +++ b/lib/bencher_schema/migrations/2026-08-26-120000_project_bmf_version/down.sql @@ -0,0 +1,59 @@ +-- project: remove bmf_version column +-- +-- SQLite drops every index a table owns along with the table, so both project +-- indexes are recreated after the swap. +PRAGMA foreign_keys = off; + +DROP INDEX IF EXISTS index_project_organization_created; + +DROP INDEX IF EXISTS index_project_not_deleted; + +CREATE TABLE down_project ( + id INTEGER PRIMARY KEY NOT NULL, + uuid TEXT NOT NULL UNIQUE, + organization_id INTEGER NOT NULL, + name TEXT NOT NULL, + slug TEXT NOT NULL UNIQUE, + url TEXT, + visibility INTEGER NOT NULL, + created BIGINT NOT NULL, + modified BIGINT NOT NULL, + deleted BIGINT, + FOREIGN KEY (organization_id) REFERENCES organization (id) ON DELETE CASCADE, + UNIQUE(organization_id, name) +); + +INSERT INTO down_project( + id, + uuid, + organization_id, + name, + slug, + url, + visibility, + created, + modified, + deleted + ) +SELECT id, + uuid, + organization_id, + name, + slug, + url, + visibility, + created, + modified, + deleted +FROM project; + +DROP TABLE project; + +ALTER TABLE down_project + RENAME TO project; + +CREATE INDEX index_project_organization_created ON project(organization_id, created); + +CREATE INDEX index_project_not_deleted ON project(id) WHERE deleted IS NULL; + +PRAGMA foreign_keys = on; diff --git a/lib/bencher_schema/migrations/2026-08-26-120000_project_bmf_version/up.sql b/lib/bencher_schema/migrations/2026-08-26-120000_project_bmf_version/up.sql new file mode 100644 index 0000000000..16d2c69dfb --- /dev/null +++ b/lib/bencher_schema/migrations/2026-08-26-120000_project_bmf_version/up.sql @@ -0,0 +1,61 @@ +-- project: add bmf_version column +-- +-- The column lives beside visibility, so the table is rebuilt around it. +-- Every project starts at 0, so a payload that declares no version reads as it +-- always has. +PRAGMA foreign_keys = off; + +DROP INDEX IF EXISTS index_project_organization_created; + +DROP INDEX IF EXISTS index_project_not_deleted; + +CREATE TABLE up_project ( + id INTEGER PRIMARY KEY NOT NULL, + uuid TEXT NOT NULL UNIQUE, + organization_id INTEGER NOT NULL, + name TEXT NOT NULL, + slug TEXT NOT NULL UNIQUE, + url TEXT, + visibility INTEGER NOT NULL, + bmf_version INTEGER NOT NULL DEFAULT 0, + created BIGINT NOT NULL, + modified BIGINT NOT NULL, + deleted BIGINT, + FOREIGN KEY (organization_id) REFERENCES organization (id) ON DELETE CASCADE, + UNIQUE(organization_id, name) +); + +INSERT INTO up_project( + id, + uuid, + organization_id, + name, + slug, + url, + visibility, + created, + modified, + deleted + ) +SELECT id, + uuid, + organization_id, + name, + slug, + url, + visibility, + created, + modified, + deleted +FROM project; + +DROP TABLE project; + +ALTER TABLE up_project + RENAME TO project; + +CREATE INDEX index_project_organization_created ON project(organization_id, created); + +CREATE INDEX index_project_not_deleted ON project(id) WHERE deleted IS NULL; + +PRAGMA foreign_keys = on; diff --git a/lib/bencher_schema/src/model/project/mod.rs b/lib/bencher_schema/src/model/project/mod.rs index d021953ebb..2fabea166a 100644 --- a/lib/bencher_schema/src/model/project/mod.rs +++ b/lib/bencher_schema/src/model/project/mod.rs @@ -1,7 +1,7 @@ use std::{string::ToString as _, sync::LazyLock}; use bencher_json::{ - DateTime, JsonNewProject, JsonProject, ProjectResourceId, ProjectSlug, ProjectUuid, + BmfVersion, DateTime, JsonNewProject, JsonProject, ProjectResourceId, ProjectSlug, ProjectUuid, ResourceName, Url, project::{JsonProjectPatch, JsonProjectPatchNull, JsonUpdateProject, ProjectRole, Visibility}, }; @@ -72,6 +72,7 @@ pub struct QueryProject { pub slug: ProjectSlug, pub url: Option, pub visibility: Visibility, + pub bmf_version: BmfVersion, pub created: DateTime, pub modified: DateTime, pub deleted: Option, @@ -595,6 +596,7 @@ impl QueryProject { slug, url, visibility, + bmf_version, created, modified, .. @@ -613,6 +615,7 @@ impl QueryProject { slug, url, visibility, + bmf_version, created, modified, claimed, @@ -740,6 +743,7 @@ impl InsertProject { slug, url, visibility, + bmf_version: BmfVersion::default(), created, modified, deleted: None, @@ -784,6 +788,7 @@ pub struct UpdateProject { pub slug: Option, pub url: Option>, pub visibility: Option, + pub bmf_version: Option, pub modified: DateTime, } @@ -796,12 +801,14 @@ impl From for UpdateProject { slug, url, visibility, + bmf_version, } = patch; Self { name, slug, url: url.map(Some), visibility, + bmf_version, modified: DateTime::now(), } }, @@ -811,12 +818,14 @@ impl From for UpdateProject { slug, url: (), visibility, + bmf_version, } = patch_url; Self { name, slug, url: Some(None), visibility, + bmf_version, modified: DateTime::now(), } }, diff --git a/lib/bencher_schema/src/model/project/report/mod.rs b/lib/bencher_schema/src/model/project/report/mod.rs index 07b7da01aa..888b1fbf39 100644 --- a/lib/bencher_schema/src/model/project/report/mod.rs +++ b/lib/bencher_schema/src/model/project/report/mod.rs @@ -227,7 +227,7 @@ impl QueryReport { let json_settings = json_report.settings.take().unwrap_or_default(); let adapter = json_settings.adapter.unwrap_or_default().normalize(); - let bmf_version = json_report.bmf_version.unwrap_or_default(); + let bmf_version = json_report.bmf_version.unwrap_or(query_project.bmf_version); // Validate job before inserting report so that report + job creation is atomic: // if OCI resolution fails, neither the report nor the job is created. @@ -243,7 +243,7 @@ impl QueryReport { new_run_job.is_claimed, new_run_job.run_job, &json_settings, - json_report.bmf_version, + bmf_version, ) .await?, ) diff --git a/lib/bencher_schema/src/model/runner/job.rs b/lib/bencher_schema/src/model/runner/job.rs index a111ddad09..f82da4087b 100644 --- a/lib/bencher_schema/src/model/runner/job.rs +++ b/lib/bencher_schema/src/model/runner/job.rs @@ -346,7 +346,7 @@ impl PendingInsertJob { is_claimed: bool, new_run_job: JsonNewRunJob, settings: &JsonReportSettings, - bmf_version: Option, + bmf_version: BmfVersion, ) -> Result { // 1. Validate registry and resolve image digest let registry_url = context.registry_url(); @@ -387,7 +387,7 @@ impl PendingInsertJob { average: settings.average, iter: new_run_job.iter, fold: settings.fold, - bmf_version, + bmf_version: Some(bmf_version), allow_failure: new_run_job.allow_failure, backdate: new_run_job.backdate, }; diff --git a/lib/bencher_schema/src/schema.rs b/lib/bencher_schema/src/schema.rs index df0cbfb58b..ca3d932e7c 100644 --- a/lib/bencher_schema/src/schema.rs +++ b/lib/bencher_schema/src/schema.rs @@ -256,6 +256,7 @@ diesel::table! { slug -> Text, url -> Nullable, visibility -> Integer, + bmf_version -> Integer, created -> BigInt, modified -> BigInt, deleted -> Nullable, diff --git a/lib/bencher_valid/src/bmf_version.rs b/lib/bencher_valid/src/bmf_version.rs index 20f5c12fd5..7427d60ba2 100644 --- a/lib/bencher_valid/src/bmf_version.rs +++ b/lib/bencher_valid/src/bmf_version.rs @@ -18,10 +18,11 @@ pub const ACCEPTED_BMF_VERSIONS: &str = "0 or 1"; // One type serves both the declared and the parsed version, so the two can be compared. /// The Bencher Metric Format (BMF) version. /// The accepted versions are 0 or 1. -/// If no version is specified, then version 0 is used. #[typeshare::typeshare] #[derive(Debug, Display, Clone, Copy, Default, Eq, PartialEq, Hash, Serialize)] #[cfg_attr(feature = "schema", derive(JsonSchema))] +#[cfg_attr(feature = "db", derive(diesel::FromSqlRow, diesel::AsExpression))] +#[cfg_attr(feature = "db", diesel(sql_type = diesel::sql_types::Integer))] pub struct BmfVersion(u8); impl TryFrom for BmfVersion { @@ -40,6 +41,12 @@ impl From for u8 { } } +impl From for i32 { + fn from(version: BmfVersion) -> Self { + Self::from(version.0) + } +} + impl BmfVersion { /// A benchmark name maps to its measures. pub const V0: Self = Self(V0_VERSION); @@ -93,6 +100,38 @@ impl Visitor<'_> for BmfVersionVisitor { } } +#[cfg(feature = "db")] +mod db { + use super::BmfVersion; + + impl diesel::serialize::ToSql for BmfVersion + where + DB: diesel::backend::Backend, + for<'a> i32: diesel::serialize::ToSql + + Into< as diesel::query_builder::BindCollector<'a, DB>>::Buffer>, + { + fn to_sql<'b>( + &'b self, + out: &mut diesel::serialize::Output<'b, '_, DB>, + ) -> diesel::serialize::Result { + out.set_value(i32::from(*self)); + Ok(diesel::serialize::IsNull::No) + } + } + + impl diesel::deserialize::FromSql for BmfVersion + where + DB: diesel::backend::Backend, + i32: diesel::deserialize::FromSql, + { + fn from_sql(bytes: DB::RawValue<'_>) -> diesel::deserialize::Result { + u8::try_from(i32::from_sql(bytes)?)? + .try_into() + .map_err(Into::into) + } + } +} + pub fn is_valid_bmf_version(version: u8) -> bool { matches!(version, V0_VERSION | V1_VERSION) } diff --git a/services/api/openapi.json b/services/api/openapi.json index 0d0b84038e..41d9c2a8dd 100644 --- a/services/api/openapi.json +++ b/services/api/openapi.json @@ -3223,7 +3223,7 @@ "projects" ], "summary": "Update a project", - "description": "Update a project. The user must have `edit` permissions for the project.", + "description": "Update a project. The user must have `edit` permissions for the project. Setting the `bmf_version` field requires a server admin.", "operationId": "project_patch", "parameters": [ { @@ -12454,7 +12454,7 @@ "minimum": 0 }, "BmfVersion": { - "description": "The Bencher Metric Format (BMF) version. The accepted versions are 0 or 1. If no version is specified, then version 0 is used.", + "description": "The Bencher Metric Format (BMF) version. The accepted versions are 0 or 1.", "type": "integer", "format": "uint8", "minimum": 0 @@ -13485,7 +13485,7 @@ }, "bmf_version": { "nullable": true, - "description": "The Bencher Metric Format (BMF) version the run declared", + "description": "The Bencher Metric Format (BMF) version the run's results are read as", "allOf": [ { "$ref": "#/components/schemas/BmfVersion" @@ -14413,7 +14413,7 @@ "properties": { "bmf_version": { "nullable": true, - "description": "The Bencher Metric Format (BMF) version this report is written in. The accepted versions are 0 or 1. If no version is specified, then version 0 is used.", + "description": "The Bencher Metric Format (BMF) version this report is written in. The accepted versions are 0 or 1. If no version is specified, then the project's `bmf_version` is used.", "allOf": [ { "$ref": "#/components/schemas/BmfVersion" @@ -14509,7 +14509,7 @@ "properties": { "bmf_version": { "nullable": true, - "description": "The Bencher Metric Format (BMF) version these results are written in. The accepted versions are 0 or 1. If no version is specified, then version 0 is used.", + "description": "The Bencher Metric Format (BMF) version these results are written in. The accepted versions are 0 or 1. If no version is specified, then the project's `bmf_version` is used.", "allOf": [ { "$ref": "#/components/schemas/BmfVersion" @@ -16168,6 +16168,9 @@ "JsonProject": { "type": "object", "properties": { + "bmf_version": { + "$ref": "#/components/schemas/BmfVersion" + }, "claimed": { "nullable": true, "allOf": [ @@ -16207,6 +16210,7 @@ } }, "required": [ + "bmf_version", "created", "modified", "name", @@ -16305,6 +16309,15 @@ "JsonProjectPatch": { "type": "object", "properties": { + "bmf_version": { + "nullable": true, + "description": "Server admin only: Set the default BMF version for the project.", + "allOf": [ + { + "$ref": "#/components/schemas/BmfVersion" + } + ] + }, "name": { "nullable": true, "description": "The new name of the project. Maximum length is 64 characters.", @@ -16346,6 +16359,14 @@ "JsonProjectPatchNull": { "type": "object", "properties": { + "bmf_version": { + "nullable": true, + "allOf": [ + { + "$ref": "#/components/schemas/BmfVersion" + } + ] + }, "name": { "nullable": true, "allOf": [ diff --git a/services/cli/src/bencher/sub/project/project/update.rs b/services/cli/src/bencher/sub/project/project/update.rs index fc9fb464a6..a3ffb246e3 100644 --- a/services/cli/src/bencher/sub/project/project/update.rs +++ b/services/cli/src/bencher/sub/project/project/update.rs @@ -62,6 +62,8 @@ impl From for JsonUpdateProject { slug: slug.map(Into::into), url: Some(url.into()), visibility, + // The field is server admin only, so the CLI never states it. + bmf_version: None, }), subtype_1: None, }, @@ -72,6 +74,7 @@ impl From for JsonUpdateProject { slug: slug.map(Into::into), url: (), visibility, + bmf_version: None, }), }, None => Self { @@ -80,6 +83,7 @@ impl From for JsonUpdateProject { slug: slug.map(Into::into), url: None, visibility, + bmf_version: None, }), subtype_1: None, }, diff --git a/services/cli/src/bencher/sub/project/report/create/mod.rs b/services/cli/src/bencher/sub/project/report/create/mod.rs index cae350168e..83de47cd1a 100644 --- a/services/cli/src/bencher/sub/project/report/create/mod.rs +++ b/services/cli/src/bencher/sub/project/report/create/mod.rs @@ -98,7 +98,8 @@ impl From for JsonNewReport { start_time, end_time, results, - // The CLI declares no BMF version yet, so the report is read as version 0. + // The CLI declares no BMF version yet, so the report is read at the + // project's default. bmf_version: None, settings: Some(JsonReportSettings { adapter, diff --git a/services/cli/src/bencher/sub/run/mod.rs b/services/cli/src/bencher/sub/run/mod.rs index 9ff337fdb2..30b82c254e 100644 --- a/services/cli/src/bencher/sub/run/mod.rs +++ b/services/cli/src/bencher/sub/run/mod.rs @@ -376,8 +376,8 @@ impl Run { start_time: start_time.into(), end_time: end_time.into(), results, - // `bencher run` declares no BMF version yet, so the results are read as - // version 0, which is how every run has always been read. + // `bencher run` declares no BMF version yet, so the results are read at + // the project's default. bmf_version: None, settings: Some(JsonReportSettings { adapter: Some(self.adapter), diff --git a/services/console/src/types/bencher.ts b/services/console/src/types/bencher.ts index 8100da6f10..b5976a9c25 100644 --- a/services/console/src/types/bencher.ts +++ b/services/console/src/types/bencher.ts @@ -13,7 +13,6 @@ export type BenchmarkResourceId = Uuid | Slug; /** * The Bencher Metric Format (BMF) version. * The accepted versions are 0 or 1. - * If no version is specified, then version 0 is used. */ export type BmfVersion = number; @@ -154,7 +153,7 @@ export interface JsonJobConfig { iter?: Iteration; /** Fold operation for combining multiple iteration results */ fold?: JsonFold; - /** The Bencher Metric Format (BMF) version the run declared */ + /** The Bencher Metric Format (BMF) version the run's results are read as */ bmf_version?: BmfVersion; /** Allow benchmark failure without short-circuiting iterations */ allow_failure?: boolean; @@ -988,6 +987,7 @@ export interface JsonProject { slug: Slug; url?: Url; visibility: Visibility; + bmf_version: BmfVersion; created: string; modified: string; claimed?: string;