From 012143e003f8dfa2e00d2287583be0767e98c771 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Wed, 17 Apr 2024 17:02:55 -0500 Subject: [PATCH 01/26] First pass on reservations --- Cargo.lock | 2 + rfd-api-spec.json | 143 ++++++++- rfd-api/src/config.rs | 9 +- rfd-api/src/context.rs | 269 +++++++++++++--- rfd-api/src/endpoints/rfd.rs | 148 +++++++-- rfd-api/src/error.rs | 2 + rfd-api/src/main.rs | 1 + rfd-api/src/permissions.rs | 4 + rfd-api/src/server.rs | 12 +- rfd-api/src/util.rs | 13 + rfd-cli/src/generated/cli.rs | 180 ++++++++++- rfd-cli/src/main.rs | 7 +- rfd-data/Cargo.toml | 1 + rfd-data/src/content/asciidoc.rs | 11 + rfd-data/src/content/markdown.rs | 11 + rfd-data/src/content/mod.rs | 24 +- rfd-data/src/content/template.rs | 61 ++++ rfd-github/Cargo.toml | 1 + rfd-github/src/lib.rs | 179 ++++++++++- rfd-processor/src/content/mod.rs | 4 + rfd-sdk/src/generated/sdk.rs | 525 +++++++++++++++++++++++++++++-- rfd-sdk/src/lib.rs | 1 + 22 files changed, 1476 insertions(+), 132 deletions(-) create mode 100644 rfd-data/src/content/template.rs diff --git a/Cargo.lock b/Cargo.lock index 6a733514..d4d08b58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2591,6 +2591,7 @@ dependencies = [ "rfd-model", "schemars", "serde", + "thiserror", ] [[package]] @@ -2602,6 +2603,7 @@ dependencies = [ "chrono", "http 0.2.11", "octorust", + "regex", "rfd-data", "thiserror", "tracing", diff --git a/rfd-api-spec.json b/rfd-api-spec.json index 90a82df6..9e18f226 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -1216,6 +1216,38 @@ "$ref": "#/components/responses/Error" } } + }, + "post": { + "summary": "Create a new RFD", + "operationId": "reserve_rfd", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReserveRfdBody" + } + } + }, + "required": true + }, + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ReserveRfdResponse" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } } }, "/rfd/{number}": { @@ -1253,7 +1285,7 @@ } }, "post": { - "operationId": "set_rfd_content", + "operationId": "set_rfd_document", "parameters": [ { "in": "path", @@ -1372,8 +1404,8 @@ "required": true }, "responses": { - "200": { - "description": "successful operation", + "202": { + "description": "successfully enqueued operation", "content": { "application/json": { "schema": { @@ -1391,6 +1423,54 @@ } } }, + "/rfd/{number}/content": { + "post": { + "operationId": "set_rfd_content", + "parameters": [ + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdUpdateContentBody" + } + } + }, + "required": true + }, + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "title": "Null", + "type": "string", + "enum": [ + null + ] + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/rfd/{number}/visibility": { "post": { "summary": "Modify the visibility of an RFD", @@ -1714,6 +1794,7 @@ "ManageMappersAll", "GetRfdsAssigned", "GetRfdsAll", + "CreateRfd", "UpdateRfdsAssigned", "UpdateRfdsAll", "ManageRfdsVisibilityAssigned", @@ -2976,6 +3057,20 @@ "kind" ] }, + { + "type": "object", + "properties": { + "kind": { + "type": "string", + "enum": [ + "CreateRfd" + ] + } + }, + "required": [ + "kind" + ] + }, { "type": "object", "properties": { @@ -4470,6 +4565,29 @@ }, "uniqueItems": true }, + "ReserveRfdBody": { + "type": "object", + "properties": { + "title": { + "type": "string" + } + }, + "required": [ + "title" + ] + }, + "ReserveRfdResponse": { + "type": "object", + "properties": { + "number": { + "type": "integer", + "format": "int32" + } + }, + "required": [ + "number" + ] + }, "Rfd": { "type": "object", "properties": { @@ -4579,10 +4697,27 @@ ] }, "RfdUpdateBody": { + "type": "object", + "properties": { + "document": { + "description": "Full Asciidoc document to store for this RFD", + "type": "string" + }, + "message": { + "nullable": true, + "description": "Optional Git commit message to send with this update (recommended)", + "type": "string" + } + }, + "required": [ + "document" + ] + }, + "RfdUpdateContentBody": { "type": "object", "properties": { "content": { - "description": "Full Asciidoc content to store for this RFD", + "description": "Asciidoc content to store for this RFD", "type": "string" }, "message": { diff --git a/rfd-api/src/config.rs b/rfd-api/src/config.rs index e1916ba3..c55ff1fe 100644 --- a/rfd-api/src/config.rs +++ b/rfd-api/src/config.rs @@ -2,9 +2,10 @@ // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at https://mozilla.org/MPL/2.0/. -use std::path::PathBuf; +use std::{collections::HashMap, path::PathBuf}; use config::{Config, ConfigError, Environment, File}; +use rfd_data::content::RfdTemplate; use secrecy::SecretString; use serde::{ de::{self, Visitor}, @@ -33,6 +34,7 @@ pub struct AppConfig { pub spec: Option, pub authn: AuthnProviders, pub search: SearchConfig, + pub content: ContentConfig, pub services: ServicesConfig, } @@ -145,6 +147,11 @@ pub struct SearchConfig { pub index: String, } +#[derive(Debug, Default, Deserialize)] +pub struct ContentConfig { + pub templates: HashMap, +} + #[derive(Debug, Deserialize)] pub struct ServicesConfig { pub github: GitHubConfig, diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index cdabe82c..d026cef1 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -15,7 +15,11 @@ use octorust::{ Client as GitHubClient, }; use partial_struct::partial; -use rfd_github::{GitHubError, GitHubRfdRepo}; +use rfd_data::{ + content::{RfdContent, RfdDocument, RfdTemplate, TemplateError}, + RfdNumber, +}; +use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, LoginAttemptState, Visibility}, storage::{ @@ -55,7 +59,9 @@ use crate::{ key::{RawApiKey, SignedApiKey}, AuthError, AuthToken, Signer, }, - config::{AsymmetricKey, GitHubAuthConfig, JwtConfig, SearchConfig, ServicesConfig}, + config::{ + AsymmetricKey, ContentConfig, GitHubAuthConfig, JwtConfig, SearchConfig, ServicesConfig, + }, endpoints::login::{ oauth::{ ClientType, OAuthProvider, OAuthProviderError, OAuthProviderFn, OAuthProviderName, @@ -128,6 +134,7 @@ pub struct ApiContext { pub secrets: SecretContext, pub oauth_providers: HashMap>, pub search: SearchContext, + pub content: ContentContext, pub github: GitHubRfdRepo, } @@ -145,6 +152,10 @@ pub struct SearchContext { pub client: SearchClient, } +pub struct ContentContext { + pub new_rfd_template: RfdTemplate, +} + pub struct RegisteredAccessToken { pub access_token: AccessToken, pub signed_token: String, @@ -194,6 +205,10 @@ pub enum UpdateRfdContentError { GitHub(#[from] GitHubError), #[error("Internal GitHub state does not currently allow for update. This commit appears as the head commit on multiple branches.")] InternalState, + #[error("Failed to construct new RFD template")] + InvalidTemplate(#[from] TemplateError), + #[error("Unable to perform action. Unable to find the default branch on GitHub.")] + NoDefaultBranch, #[error(transparent)] Storage(#[from] StoreError), } @@ -239,6 +254,7 @@ impl ApiContext { jwt: JwtConfig, keys: Vec, search: SearchConfig, + content: ContentConfig, services: ServicesConfig, ) -> Result { let mut jwt_signers = vec![]; @@ -305,6 +321,13 @@ impl ApiContext { search: SearchContext { client: SearchClient::new(search.host, search.index, search.key), }, + content: ContentContext { + new_rfd_template: content + .templates + .get("new") + .cloned() + .ok_or(AppError::MissingNewRfdTemplate)?, + }, github: GitHubRfdRepo::new( &match services.github.auth { GitHubAuthConfig::Installation { @@ -664,6 +687,73 @@ impl ApiContext { Ok(rfd_list) } + #[instrument(skip(self, caller), err(Debug))] + pub async fn create_rfd( + &self, + caller: &ApiCaller, + title: String, + ) -> ResourceResult { + if caller.can(&ApiPermission::CreateRfd) { + tracing::info!("Reserving new RFD"); + + // We acknowledge that there are race conditions here, as there would be if an end user + // were to attempt to reserve an RFD number manually + let GitHubNewRfdNumber { + number: next_rfd_number, + commit, + } = self + .github + .next_rfd_number() + .await + .map_err(UpdateRfdContentError::GitHub) + .to_resource_result()?; + + tracing::info!(?next_rfd_number, commit, "Creating new RFD branch"); + + // Branch off of the default branch with a new branch with the padded form of the RFD number + self.github + .create_branch(&next_rfd_number, &commit) + .await + .map_err(UpdateRfdContentError::GitHub) + .to_resource_result()?; + + tracing::info!( + ?next_rfd_number, + ?commit, + "Created new branch for reserving RFD off of default branch" + ); + + let content = self + .content + .new_rfd_template + .clone() + .field("number".to_string(), next_rfd_number.to_string()) + .field("title".to_string(), title) + .build() + .map_err(UpdateRfdContentError::InvalidTemplate) + .to_resource_result()?; + + self.commit_rfd_document( + caller, + next_rfd_number.into(), + &content.render(), + Some("Reserving RFD number"), + &commit, + Some(&next_rfd_number.as_number_string()), + ) + .await?; + + tracing::info!( + ?next_rfd_number, + "Pushed placeholder RFD to reserved branch" + ); + + Ok(next_rfd_number) + } else { + Err(ResourceError::Restricted) + } + } + #[instrument(skip(self, caller))] pub async fn get_rfd( &self, @@ -779,17 +869,14 @@ impl ApiContext { } } - #[instrument(skip(self, caller, content))] - pub async fn update_rfd_content( + async fn get_latest_rfd_revision( &self, caller: &ApiCaller, rfd_number: i32, - content: &str, - message: Option<&str>, - ) -> ResourceResult<(), UpdateRfdContentError> { + ) -> ResourceResult { if caller.any(&[ - &ApiPermission::UpdateRfd(rfd_number), - &ApiPermission::UpdateRfdsAll, + &ApiPermission::GetRfd(rfd_number), + &ApiPermission::GetRfdsAll, ]) { let rfds = RfdStore::list( &*self.storage, @@ -797,8 +884,8 @@ impl ApiContext { &ListPagination::default().limit(1), ) .await - .map_err(UpdateRfdContentError::Storage) .to_resource_result()?; + if let Some(rfd) = rfds.into_iter().nth(0) { let revisions = RfdRevisionStore::list( &*self.storage, @@ -806,48 +893,11 @@ impl ApiContext { &ListPagination::default().limit(1), ) .await - .map_err(UpdateRfdContentError::Storage) .to_resource_result()?; - let latest_revision = match revisions.into_iter().nth(0) { + match revisions.into_iter().nth(0) { Some(revision) => Ok(revision), None => Err(ResourceError::DoesNotExist), - }?; - - let sha = latest_revision.commit_sha; - let mut github_locations = self - .github - .locations_for_commit(sha.clone()) - .await - .map_err(UpdateRfdContentError::GitHub) - .to_resource_result()?; - - match github_locations.len() { - 0 => { - tracing::warn!( - sha, - rfd_number, - "Failed to find a GitHub location for most recent revision" - ); - Err(ResourceError::DoesNotExist) - } - 1 => { - let message = format!( - "{}\n\nSubmitted by {}", - message.unwrap_or("RFD API update"), - caller.id - ); - - // Unwrap is checked by the location length - let location = github_locations.pop().unwrap(); - location - .upsert(&rfd_number.into(), content.as_bytes(), &message) - .await - .map_err(UpdateRfdContentError::GitHub) - .to_resource_result()?; - Ok(()) - } - _ => Err(UpdateRfdContentError::InternalState).to_resource_result(), } } else { Err(ResourceError::DoesNotExist) @@ -857,6 +907,124 @@ impl ApiContext { } } + #[instrument(skip(self, caller, content))] + pub async fn update_rfd_content( + &self, + caller: &ApiCaller, + rfd_number: i32, + content: &str, + message: Option<&str>, + branch_name: Option<&str>, + ) -> ResourceResult<(), UpdateRfdContentError> { + if caller.any(&[ + &ApiPermission::UpdateRfd(rfd_number), + &ApiPermission::UpdateRfdsAll, + ]) { + let latest_revision = self + .get_latest_rfd_revision(caller, rfd_number) + .await + .map_err(|err| err.inner_into())?; + + let sha = latest_revision.commit_sha.clone(); + let mut updated_content: RfdContent = latest_revision.into(); + updated_content.update_body(content); + + self.commit_rfd_document( + caller, + rfd_number, + updated_content.raw(), + message, + &sha, + branch_name, + ) + .await + } else { + resource_restricted() + } + } + + #[instrument(skip(self, caller, document))] + pub async fn update_rfd_document( + &self, + caller: &ApiCaller, + rfd_number: i32, + document: &str, + message: Option<&str>, + branch_name: Option<&str>, + ) -> ResourceResult<(), UpdateRfdContentError> { + if caller.any(&[ + &ApiPermission::UpdateRfd(rfd_number), + &ApiPermission::UpdateRfdsAll, + ]) { + let latest_revision = self + .get_latest_rfd_revision(caller, rfd_number) + .await + .map_err(|err| err.inner_into())?; + + let sha = latest_revision.commit_sha; + self.commit_rfd_document(caller, rfd_number, document, message, &sha, branch_name) + .await + } else { + resource_restricted() + } + } + + #[instrument(skip(self, caller, document))] + async fn commit_rfd_document( + &self, + caller: &ApiCaller, + rfd_number: i32, + document: &str, + message: Option<&str>, + head: &str, + branch_name: Option<&str>, + ) -> ResourceResult<(), UpdateRfdContentError> { + let mut github_locations = self + .github + .locations_for_commit(head.to_string()) + .await + .map_err(UpdateRfdContentError::GitHub) + .to_resource_result()? + .into_iter() + .filter(|location| { + branch_name + .as_ref() + .map(|branch_name| branch_name == &location.branch) + .unwrap_or(true) + }) + .collect::>(); + + match github_locations.len() { + 0 => { + tracing::warn!( + head, + rfd_number, + "Failed to find a GitHub location for most recent revision" + ); + Err(ResourceError::DoesNotExist) + } + 1 => { + tracing::info!("Pushing RFD commit to GitHub"); + + let message = format!( + "{}\n\nSubmitted by {}", + message.unwrap_or("RFD API update"), + caller.id + ); + + // Unwrap is checked by the location length + let location = github_locations.pop().unwrap(); + location + .upsert(&rfd_number.into(), document.as_bytes(), &message) + .await + .map_err(UpdateRfdContentError::GitHub) + .to_resource_result()?; + Ok(()) + } + _ => Err(UpdateRfdContentError::InternalState).to_resource_result(), + } + } + #[instrument(skip(self, caller))] pub async fn update_rfd_visibility( &self, @@ -2074,7 +2242,9 @@ pub(crate) mod test_mocks { use w_api_permissions::Caller; use crate::{ - config::{GitHubAuthConfig, GitHubConfig, JwtConfig, SearchConfig, ServicesConfig}, + config::{ + ContentConfig, GitHubAuthConfig, GitHubConfig, JwtConfig, SearchConfig, ServicesConfig, + }, endpoints::login::oauth::{google::GoogleOAuthProvider, OAuthProviderName}, permissions::ApiPermission, util::tests::mock_key, @@ -2093,6 +2263,7 @@ pub(crate) mod test_mocks { mock_key(), ], SearchConfig::default(), + ContentConfig::default(), ServicesConfig { github: GitHubConfig { owner: String::new(), diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index dffa2c6b..f84133f6 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -29,12 +29,6 @@ use crate::{ ApiCaller, }; -#[derive(Debug, Deserialize, JsonSchema)] -pub struct RfdPathParams { - /// The RFD number (examples: 1 or 123) - number: String, -} - /// List all available RFDs #[trace_request] #[endpoint { @@ -59,6 +53,55 @@ async fn get_rfds_op( Ok(HttpResponseOk(rfds)) } +#[derive(Debug, Deserialize, JsonSchema)] +pub struct ReserveRfdBody { + pub title: String, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct ReserveRfdResponse { + number: i32, +} + +/// Create a new RFD +#[trace_request] +#[endpoint { + method = POST, + path = "/rfd", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn reserve_rfd( + rqctx: RequestContext, + body: TypedBody, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let auth = ctx.authn_token(&rqctx).await?; + reserve_rfd_op( + ctx, + &ctx.get_caller(auth.as_ref()).await?, + body.into_inner(), + ) + .await +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn reserve_rfd_op( + ctx: &ApiContext, + caller: &ApiCaller, + body: ReserveRfdBody, +) -> Result, HttpError> { + let number = ctx.create_rfd(caller, body.title).await?; + Ok(HttpResponseAccepted(ReserveRfdResponse { + number: number.into(), + })) +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdPathParams { + /// The RFD number (examples: 1 or 123) + number: String, +} + /// Get the latest representation of an RFD #[trace_request] #[endpoint { @@ -98,8 +141,8 @@ async fn get_rfd_op( #[derive(Debug, Deserialize, JsonSchema)] pub struct RfdUpdateBody { - /// Full Asciidoc content to store for this RFD - content: String, + /// Full Asciidoc document to store for this RFD + document: String, /// Optional Git commit message to send with this update (recommended) message: Option, } @@ -109,10 +152,63 @@ pub struct RfdUpdateBody { method = POST, path = "/rfd/{number}", }] -pub async fn set_rfd_content( +pub async fn set_rfd_document( rqctx: RequestContext, path: Path, body: TypedBody, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let auth = ctx.authn_token(&rqctx).await?; + set_rfd_document_op( + ctx, + &ctx.get_caller(auth.as_ref()).await?, + path.into_inner().number, + body.into_inner(), + ) + .await +} + +async fn set_rfd_document_op( + ctx: &ApiContext, + caller: &ApiCaller, + number: String, + body: RfdUpdateBody, +) -> Result, HttpError> { + if let Ok(rfd_number) = number.parse::() { + ctx.update_rfd_document( + caller, + rfd_number.into(), + &body.document, + body.message.as_deref(), + None, + ) + .await?; + Ok(HttpResponseAccepted(())) + } else { + Err(client_error( + StatusCode::BAD_REQUEST, + "Malformed RFD number", + )) + } +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdUpdateContentBody { + /// Asciidoc content to store for this RFD + content: String, + /// Optional Git commit message to send with this update (recommended) + message: Option, +} + +#[trace_request] +#[endpoint { + method = POST, + path = "/rfd/{number}/content", +}] +pub async fn set_rfd_content( + rqctx: RequestContext, + path: Path, + body: TypedBody, ) -> Result, HttpError> { let ctx = rqctx.context(); let auth = ctx.authn_token(&rqctx).await?; @@ -129,14 +225,15 @@ async fn set_rfd_content_op( ctx: &ApiContext, caller: &ApiCaller, number: String, - body: RfdUpdateBody, + body: RfdUpdateContentBody, ) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { - ctx.update_rfd_content( + ctx.update_rfd_document( caller, rfd_number.into(), &body.content, body.message.as_deref(), + None, ) .await?; Ok(HttpResponseAccepted(())) @@ -207,7 +304,7 @@ async fn get_rfd_attr_op( ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(rfd.content)), }; - extract_attr(&attr, &content) + extract_attr(&attr, &content).map(HttpResponseOk) } else { Err(client_error( StatusCode::BAD_REQUEST, @@ -235,7 +332,7 @@ pub async fn set_rfd_attr( rqctx: RequestContext, path: Path, body: TypedBody, -) -> Result, HttpError> { +) -> Result, HttpError> { let ctx = rqctx.context(); let auth = ctx.authn_token(&rqctx).await?; let path = path.into_inner(); @@ -256,7 +353,7 @@ async fn set_rfd_attr_op( number: String, attr: RfdAttrName, body: &RfdAttrValue, -) -> Result, HttpError> { +) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { // Get the latest revision let revision = ctx.get_rfd_revision(caller, rfd_number, None).await?; @@ -283,10 +380,16 @@ async fn set_rfd_attr_op( // Persist the data back to GitHub. Note that we do not store this back to the database. // We rely on GitHub as the source of truth and revisions are required to tbe linked to // commits - ctx.update_rfd_content(caller, rfd_number, content.raw(), body.message.as_deref()) - .await?; + ctx.update_rfd_document( + caller, + rfd_number, + content.raw(), + body.message.as_deref(), + None, + ) + .await?; - extract_attr(&attr, &content) + extract_attr(&attr, &content).map(HttpResponseAccepted) } else { Err(client_error( StatusCode::BAD_REQUEST, @@ -295,10 +398,7 @@ async fn set_rfd_attr_op( } } -fn extract_attr( - attr: &RfdAttrName, - content: &RfdContent, -) -> Result, HttpError> { +fn extract_attr(attr: &RfdAttrName, content: &RfdContent) -> Result { match attr { RfdAttrName::Discussion => content .get_discussion() @@ -308,7 +408,7 @@ fn extract_attr( "RFD does not have the requested attribute".to_string(), ) }) - .map(|value| HttpResponseOk(RfdAttr::Discussion(value.to_string()))), + .map(|value| RfdAttr::Discussion(value.to_string())), RfdAttrName::Labels => content .get_labels() .ok_or_else(|| { @@ -317,7 +417,7 @@ fn extract_attr( "RFD does not have the requested attribute".to_string(), ) }) - .map(|value| HttpResponseOk(RfdAttr::Labels(value.to_string()))), + .map(|value| RfdAttr::Labels(value.to_string())), RfdAttrName::State => content .get_state() .ok_or_else(|| { @@ -327,7 +427,7 @@ fn extract_attr( ) }) .and_then(|value| match value.try_into() { - Ok(rfd_state) => Ok(HttpResponseOk(RfdAttr::State(rfd_state))), + Ok(rfd_state) => Ok(RfdAttr::State(rfd_state)), Err(err) => { tracing::error!(?err, "RFD has an invalid state stored in its contents"); Err(HttpError::for_internal_error( diff --git a/rfd-api/src/error.rs b/rfd-api/src/error.rs index f96377af..ab33952c 100644 --- a/rfd-api/src/error.rs +++ b/rfd-api/src/error.rs @@ -23,6 +23,8 @@ pub enum AppError { GitHub(#[from] GitHubError), #[error("Invalid GitHub private key")] InvalidGitHubPrivateKey(#[from] rsa::pkcs1::Error), + #[error("A template for new RFDs must be defined")] + MissingNewRfdTemplate, #[error("At least one JWT signing key must be configured")] NoConfiguredJwtKeys, #[error("Failed to construct GitHub client")] diff --git a/rfd-api/src/main.rs b/rfd-api/src/main.rs index a4b5db9a..79db7dd6 100644 --- a/rfd-api/src/main.rs +++ b/rfd-api/src/main.rs @@ -83,6 +83,7 @@ async fn main() -> anyhow::Result<()> { config.jwt, config.keys, config.search, + config.content, config.services, ) .await?; diff --git a/rfd-api/src/permissions.rs b/rfd-api/src/permissions.rs index df3a91ef..0d12bf73 100644 --- a/rfd-api/src/permissions.rs +++ b/rfd-api/src/permissions.rs @@ -81,6 +81,7 @@ pub enum ApiPermission { GetRfds(BTreeSet), GetRfdsAssigned, GetRfdsAll, + CreateRfd, UpdateRfd(i32), UpdateRfds(BTreeSet), UpdateRfdsAssigned, @@ -175,6 +176,8 @@ impl ApiPermission { ApiPermission::GetRfdsAssigned => "rfd:content:r", ApiPermission::GetRfdsAll => "rfd:content:r", + ApiPermission::CreateRfd => "rfd:content:w", + ApiPermission::UpdateRfd(_) => "rfd:content:w", ApiPermission::UpdateRfds(_) => "rfd:content:w", ApiPermission::UpdateRfdsAssigned => "rfd:content:w", @@ -276,6 +279,7 @@ impl ApiPermission { permissions.insert(ApiPermission::GetRfdsAll); } "rfd:content:w" => { + permissions.insert(ApiPermission::CreateRfd); permissions.insert(ApiPermission::UpdateRfdsAssigned); permissions.insert(ApiPermission::UpdateRfdsAll); } diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index 3daee1d6..20ef28ab 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -30,8 +30,8 @@ use crate::{ }, mappers::{create_mapper, delete_mapper, get_mappers}, rfd::{ - get_rfd, get_rfd_attr, get_rfds, search_rfds, set_rfd_attr, set_rfd_content, - update_rfd_visibility, + get_rfd, get_rfd_attr, get_rfds, reserve_rfd, search_rfds, set_rfd_attr, + set_rfd_content, set_rfd_document, update_rfd_visibility, }, webhook::github_webhook, well_known::{jwks_json, openid_configuration}, @@ -91,16 +91,20 @@ pub fn server( // RFDs api.register(get_rfds).expect("Failed to register endpoint"); api.register(get_rfd).expect("Failed to register endpoint"); + api.register(reserve_rfd) + .expect("Failed to register endpoint"); + api.register(set_rfd_document) + .expect("Failed to register endpoint"); api.register(set_rfd_content) .expect("Failed to register endpoint"); api.register(get_rfd_attr) .expect("Failed to register endpoint"); api.register(set_rfd_attr) .expect("Failed to register endpoint"); - api.register(search_rfds) - .expect("Failed to register endpoint"); api.register(update_rfd_visibility) .expect("Failed to register endpoint"); + api.register(search_rfds) + .expect("Failed to register endpoint"); // Webhooks api.register(github_webhook) diff --git a/rfd-api/src/util.rs b/rfd-api/src/util.rs index d8768e41..46699887 100644 --- a/rfd-api/src/util.rs +++ b/rfd-api/src/util.rs @@ -108,6 +108,19 @@ pub mod response { InternalError(#[source] E), } + impl ResourceError { + pub fn inner_into(self) -> ResourceError + where + F: From, + { + match self { + ResourceError::DoesNotExist => ResourceError::DoesNotExist, + ResourceError::Restricted => ResourceError::Restricted, + ResourceError::InternalError(inner) => ResourceError::InternalError(inner.into()), + } + } + } + pub trait ToResourceResultOpt { fn opt_to_resource_result(self) -> ResourceResult; } diff --git a/rfd-cli/src/generated/cli.rs b/rfd-cli/src/generated/cli.rs index 775ce404..b843bdee 100644 --- a/rfd-cli/src/generated/cli.rs +++ b/rfd-cli/src/generated/cli.rs @@ -51,10 +51,12 @@ impl Cli { CliCommand::CreateOauthClientSecret => Self::cli_create_oauth_client_secret(), CliCommand::DeleteOauthClientSecret => Self::cli_delete_oauth_client_secret(), CliCommand::GetRfds => Self::cli_get_rfds(), + CliCommand::ReserveRfd => Self::cli_reserve_rfd(), CliCommand::GetRfd => Self::cli_get_rfd(), - CliCommand::SetRfdContent => Self::cli_set_rfd_content(), + CliCommand::SetRfdDocument => Self::cli_set_rfd_document(), CliCommand::GetRfdAttr => Self::cli_get_rfd_attr(), CliCommand::SetRfdAttr => Self::cli_set_rfd_attr(), + CliCommand::SetRfdContent => Self::cli_set_rfd_content(), CliCommand::UpdateRfdVisibility => Self::cli_update_rfd_visibility(), CliCommand::SearchRfds => Self::cli_search_rfds(), CliCommand::GetSelf => Self::cli_get_self(), @@ -694,6 +696,31 @@ impl Cli { clap::Command::new("").about("List all available RFDs") } + pub fn cli_reserve_rfd() -> clap::Command { + clap::Command::new("") + .arg( + clap::Arg::new("title") + .long("title") + .value_parser(clap::value_parser!(String)) + .required_unless_present("json-body"), + ) + .arg( + clap::Arg::new("json-body") + .long("json-body") + .value_name("JSON-FILE") + .required(false) + .value_parser(clap::value_parser!(std::path::PathBuf)) + .help("Path to a file that contains the full json body."), + ) + .arg( + clap::Arg::new("json-body-template") + .long("json-body-template") + .action(clap::ArgAction::SetTrue) + .help("XXX"), + ) + .about("Create a new RFD") + } + pub fn cli_get_rfd() -> clap::Command { clap::Command::new("") .arg( @@ -706,14 +733,14 @@ impl Cli { .about("Get the latest representation of an RFD") } - pub fn cli_set_rfd_content() -> clap::Command { + pub fn cli_set_rfd_document() -> clap::Command { clap::Command::new("") .arg( - clap::Arg::new("content") - .long("content") + clap::Arg::new("document") + .long("document") .value_parser(clap::value_parser!(String)) .required_unless_present("json-body") - .help("Full Asciidoc content to store for this RFD"), + .help("Full Asciidoc document to store for this RFD"), ) .arg( clap::Arg::new("message") @@ -821,6 +848,45 @@ impl Cli { .about("Set an attribute of a given RFD") } + pub fn cli_set_rfd_content() -> clap::Command { + clap::Command::new("") + .arg( + clap::Arg::new("content") + .long("content") + .value_parser(clap::value_parser!(String)) + .required_unless_present("json-body") + .help("Asciidoc content to store for this RFD"), + ) + .arg( + clap::Arg::new("message") + .long("message") + .value_parser(clap::value_parser!(String)) + .required(false) + .help("Optional Git commit message to send with this update (recommended)"), + ) + .arg( + clap::Arg::new("number") + .long("number") + .value_parser(clap::value_parser!(String)) + .required(true) + .help("The RFD number (examples: 1 or 123)"), + ) + .arg( + clap::Arg::new("json-body") + .long("json-body") + .value_name("JSON-FILE") + .required(false) + .value_parser(clap::value_parser!(std::path::PathBuf)) + .help("Path to a file that contains the full json body."), + ) + .arg( + clap::Arg::new("json-body-template") + .long("json-body-template") + .action(clap::ArgAction::SetTrue) + .help("XXX"), + ) + } + pub fn cli_update_rfd_visibility() -> clap::Command { clap::Command::new("") .arg( @@ -948,10 +1014,12 @@ impl Cli { self.execute_delete_oauth_client_secret(matches).await } CliCommand::GetRfds => self.execute_get_rfds(matches).await, + CliCommand::ReserveRfd => self.execute_reserve_rfd(matches).await, CliCommand::GetRfd => self.execute_get_rfd(matches).await, - CliCommand::SetRfdContent => self.execute_set_rfd_content(matches).await, + CliCommand::SetRfdDocument => self.execute_set_rfd_document(matches).await, CliCommand::GetRfdAttr => self.execute_get_rfd_attr(matches).await, CliCommand::SetRfdAttr => self.execute_set_rfd_attr(matches).await, + CliCommand::SetRfdContent => self.execute_set_rfd_content(matches).await, CliCommand::UpdateRfdVisibility => self.execute_update_rfd_visibility(matches).await, CliCommand::SearchRfds => self.execute_search_rfds(matches).await, CliCommand::GetSelf => self.execute_get_self(matches).await, @@ -1819,6 +1887,32 @@ impl Cli { } } + pub async fn execute_reserve_rfd(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { + let mut request = self.client.reserve_rfd(); + if let Some(value) = matches.get_one::("title") { + request = request.body_map(|body| body.title(value.clone())) + } + + if let Some(value) = matches.get_one::("json-body") { + let body_txt = std::fs::read_to_string(value).unwrap(); + let body_value = serde_json::from_str::(&body_txt).unwrap(); + request = request.body(body_value); + } + + self.config.execute_reserve_rfd(matches, &mut request)?; + let result = request.send().await; + match result { + Ok(r) => { + self.config.item_success(&r); + Ok(()) + } + Err(r) => { + self.config.item_error(&r); + Err(anyhow::Error::new(r)) + } + } + } + pub async fn execute_get_rfd(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { let mut request = self.client.get_rfd(); if let Some(value) = matches.get_one::("number") { @@ -1839,10 +1933,10 @@ impl Cli { } } - pub async fn execute_set_rfd_content(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { - let mut request = self.client.set_rfd_content(); - if let Some(value) = matches.get_one::("content") { - request = request.body_map(|body| body.content(value.clone())) + pub async fn execute_set_rfd_document(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { + let mut request = self.client.set_rfd_document(); + if let Some(value) = matches.get_one::("document") { + request = request.body_map(|body| body.document(value.clone())) } if let Some(value) = matches.get_one::("message") { @@ -1859,7 +1953,8 @@ impl Cli { request = request.body(body_value); } - self.config.execute_set_rfd_content(matches, &mut request)?; + self.config + .execute_set_rfd_document(matches, &mut request)?; let result = request.send().await; match result { Ok(r) => { @@ -1935,6 +2030,41 @@ impl Cli { } } + pub async fn execute_set_rfd_content(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { + let mut request = self.client.set_rfd_content(); + if let Some(value) = matches.get_one::("content") { + request = request.body_map(|body| body.content(value.clone())) + } + + if let Some(value) = matches.get_one::("message") { + request = request.body_map(|body| body.message(value.clone())) + } + + if let Some(value) = matches.get_one::("number") { + request = request.number(value.clone()); + } + + if let Some(value) = matches.get_one::("json-body") { + let body_txt = std::fs::read_to_string(value).unwrap(); + let body_value = + serde_json::from_str::(&body_txt).unwrap(); + request = request.body(body_value); + } + + self.config.execute_set_rfd_content(matches, &mut request)?; + let result = request.send().await; + match result { + Ok(r) => { + self.config.item_success(&r); + Ok(()) + } + Err(r) => { + self.config.item_error(&r); + Err(anyhow::Error::new(r)) + } + } + } + pub async fn execute_update_rfd_visibility( &self, matches: &clap::ArgMatches, @@ -2301,6 +2431,14 @@ pub trait CliConfig { Ok(()) } + fn execute_reserve_rfd( + &self, + matches: &clap::ArgMatches, + request: &mut builder::ReserveRfd, + ) -> anyhow::Result<()> { + Ok(()) + } + fn execute_get_rfd( &self, matches: &clap::ArgMatches, @@ -2309,10 +2447,10 @@ pub trait CliConfig { Ok(()) } - fn execute_set_rfd_content( + fn execute_set_rfd_document( &self, matches: &clap::ArgMatches, - request: &mut builder::SetRfdContent, + request: &mut builder::SetRfdDocument, ) -> anyhow::Result<()> { Ok(()) } @@ -2333,6 +2471,14 @@ pub trait CliConfig { Ok(()) } + fn execute_set_rfd_content( + &self, + matches: &clap::ArgMatches, + request: &mut builder::SetRfdContent, + ) -> anyhow::Result<()> { + Ok(()) + } + fn execute_update_rfd_visibility( &self, matches: &clap::ArgMatches, @@ -2392,10 +2538,12 @@ pub enum CliCommand { CreateOauthClientSecret, DeleteOauthClientSecret, GetRfds, + ReserveRfd, GetRfd, - SetRfdContent, + SetRfdDocument, GetRfdAttr, SetRfdAttr, + SetRfdContent, UpdateRfdVisibility, SearchRfds, GetSelf, @@ -2436,10 +2584,12 @@ impl CliCommand { CliCommand::CreateOauthClientSecret, CliCommand::DeleteOauthClientSecret, CliCommand::GetRfds, + CliCommand::ReserveRfd, CliCommand::GetRfd, - CliCommand::SetRfdContent, + CliCommand::SetRfdDocument, CliCommand::GetRfdAttr, CliCommand::SetRfdAttr, + CliCommand::SetRfdContent, CliCommand::UpdateRfdVisibility, CliCommand::SearchRfds, CliCommand::GetSelf, diff --git a/rfd-cli/src/main.rs b/rfd-cli/src/main.rs index 2ea2e60f..0363581c 100644 --- a/rfd-cli/src/main.rs +++ b/rfd-cli/src/main.rs @@ -136,9 +136,11 @@ fn cmd_path<'a>(cmd: &CliCommand) -> Option<&'a str> { CliCommand::GetRfds => Some("list"), CliCommand::GetRfdAttr => Some("attr"), CliCommand::SearchRfds => Some("search"), + CliCommand::ReserveRfd => Some("reserve"), CliCommand::SetRfdAttr => Some("edit attr"), CliCommand::SetRfdContent => Some("edit content"), + CliCommand::SetRfdDocument => Some("edit document"), CliCommand::UpdateRfdVisibility => Some("edit visibility"), // User commands @@ -369,7 +371,10 @@ impl ProgenitorCliConfig for Context { .unwrap() .output_search_results(reserialize(value)), "RfdAttr" => self.printer().unwrap().output_rfd_attr(reserialize(value)), - _ => eprintln!("Unhandled response"), + other => eprintln!( + "Unhandled response type: {}. Please report this as a bug.", + other + ), } } diff --git a/rfd-data/Cargo.toml b/rfd-data/Cargo.toml index 3a7a9465..411f35bc 100644 --- a/rfd-data/Cargo.toml +++ b/rfd-data/Cargo.toml @@ -10,3 +10,4 @@ regex = { workspace = true } rfd-model = { path = "../rfd-model" } schemars = { workspace = true } serde = { workspace = true } +thiserror = { workspace = true } diff --git a/rfd-data/src/content/asciidoc.rs b/rfd-data/src/content/asciidoc.rs index 9fd4235b..9f2b62c8 100644 --- a/rfd-data/src/content/asciidoc.rs +++ b/rfd-data/src/content/asciidoc.rs @@ -150,6 +150,17 @@ impl<'a> RfdDocument for RfdAsciidoc<'a> { self.title_pattern().splitn(&self.content, 2).nth(1) } + fn update_body(&mut self, value: &str) { + self.content = Cow::Owned( + self.title_pattern() + .splitn(&self.content, 2) + .nth(0) + .unwrap_or_default() + .to_string() + + value, + ) + } + /// Get a reference to the internal unparsed contents fn raw(&self) -> &str { &self.content diff --git a/rfd-data/src/content/markdown.rs b/rfd-data/src/content/markdown.rs index dc96fe98..6a694508 100644 --- a/rfd-data/src/content/markdown.rs +++ b/rfd-data/src/content/markdown.rs @@ -119,6 +119,17 @@ impl<'a> RfdDocument for RfdMarkdown<'a> { self.title_pattern().splitn(&self.content, 2).nth(1) } + fn update_body(&mut self, value: &str) { + self.content = Cow::Owned( + self.title_pattern() + .splitn(&self.content, 2) + .nth(0) + .unwrap_or_default() + .to_string() + + value, + ) + } + /// Get a reference to the internal unparsed contents fn raw(&self) -> &str { &self.content diff --git a/rfd-data/src/content/mod.rs b/rfd-data/src/content/mod.rs index 2118904d..38cf9250 100644 --- a/rfd-data/src/content/mod.rs +++ b/rfd-data/src/content/mod.rs @@ -4,10 +4,13 @@ mod asciidoc; mod markdown; +mod template; pub use asciidoc::RfdAsciidoc; pub use markdown::RfdMarkdown; -use rfd_model::schema_ext::ContentFormat; +pub use template::{RenderableRfdTemplate, RfdTemplate, TemplateError}; + +use rfd_model::{schema_ext::ContentFormat, RfdRevision}; pub trait RfdDocument { /// Extract the title from the internal content @@ -41,6 +44,9 @@ pub trait RfdDocument { /// Get a reference to the contents of the RFD body fn body(&self) -> Option<&str>; + /// Get a reference to the contents of the RFD body + fn update_body(&mut self, value: &str); + /// Get a reference to the internal unparsed contents fn raw(&self) -> &str; } @@ -131,6 +137,13 @@ impl<'a> RfdDocument for RfdContent<'a> { } } + fn update_body(&mut self, value: &str) { + match self { + Self::Asciidoc(inner) => inner.update_body(value), + Self::Markdown(inner) => inner.update_body(value), + } + } + fn raw(&self) -> &str { match self { Self::Asciidoc(inner) => inner.raw(), @@ -138,3 +151,12 @@ impl<'a> RfdDocument for RfdContent<'a> { } } } + +impl<'a> From for RfdContent<'a> { + fn from(value: RfdRevision) -> Self { + match value.content_format { + ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(value.content)), + ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(value.content)), + } + } +} diff --git a/rfd-data/src/content/template.rs b/rfd-data/src/content/template.rs new file mode 100644 index 00000000..2a6d508f --- /dev/null +++ b/rfd-data/src/content/template.rs @@ -0,0 +1,61 @@ +use serde::{Deserialize, Serialize}; +use std::collections::HashMap; +use thiserror::Error; + +#[derive(Debug, Error)] +pub enum TemplateError { + #[error("Template is missing some of the required values")] + MissingRequiredFields { + template: RfdTemplate, + values: Vec, + }, +} + +#[derive(Clone, Debug, Deserialize, Serialize)] +pub struct RfdTemplate { + template: String, + #[serde(default)] + values: HashMap, + required_fields: Vec, +} + +#[derive(Clone, Debug)] +pub struct RenderableRfdTemplate(RfdTemplate); + +impl RfdTemplate { + pub fn field(mut self, field: String, value: String) -> Self { + self.values.insert(field, value); + self + } + + pub fn build(self) -> Result { + let set_fields = self.values.keys().collect::>(); + let missing_fields = self + .required_fields + .iter() + .filter(|field| !set_fields.contains(field)) + .cloned() + .collect::>(); + + if missing_fields.len() == 0 { + Ok(RenderableRfdTemplate(self)) + } else { + Err(TemplateError::MissingRequiredFields { + template: self, + values: missing_fields, + }) + } + } +} + +impl RenderableRfdTemplate { + pub fn render(self) -> String { + let mut rendered = self.0.template; + + for field in self.0.required_fields { + rendered = rendered.replace(&format!("{{{}}}", field), self.0.values.get(&field).expect("Renderable template is missing a required field. This is a bug, please report.")); + } + + rendered + } +} diff --git a/rfd-github/Cargo.toml b/rfd-github/Cargo.toml index 8751363f..7ab2f685 100644 --- a/rfd-github/Cargo.toml +++ b/rfd-github/Cargo.toml @@ -11,6 +11,7 @@ base64 = { workspace = true } chrono = { workspace = true } http = { workspace = true } octorust = { workspace = true } +regex = { workspace = true } rfd-data = { path = "../rfd-data" } thiserror = { workspace = true } tracing = { workspace = true } \ No newline at end of file diff --git a/rfd-github/src/lib.rs b/rfd-github/src/lib.rs index 83ad4570..7f7d3562 100644 --- a/rfd-github/src/lib.rs +++ b/rfd-github/src/lib.rs @@ -15,9 +15,10 @@ use base64::{prelude::BASE64_STANDARD, DecodeError, Engine}; use chrono::{DateTime, ParseError, Utc}; use http::StatusCode; use octorust::{ - types::{PullRequestSimple, ReposCreateUpdateFileContentsRequest}, + types::{GitCreateRefRequest, PullRequestSimple, ReposCreateUpdateFileContentsRequest}, Client, ClientError, Response, }; +use regex::Regex; use rfd_data::{ content::{RfdAsciidoc, RfdContent, RfdMarkdown}, RfdNumber, @@ -86,6 +87,120 @@ impl GitHubRfdRepo { }) } + #[instrument(skip(self))] + pub async fn next_rfd_number(&self) -> Result { + // We need to use two separate calls to try to determine the next available RFD number: + // `1. The list of published RFD files that exist on the default branch + // 2. The list of open branches in the RFD repo that are not yet published + + // Get the latest commit of the default branch + let default = self + .client + .repos() + .get_branch(&self.owner, &self.repo, &self.default_branch) + .await? + .body; + + tracing::debug!(?default.commit.sha, "Fetched default branch"); + + // List all of the files that are in the RFD path of the repo + let rfd_files = match self + .client + .repos() + .get_content_vec_entries(&self.owner, &self.repo, &self.path, &default.commit.sha) + .await + { + Ok(entries) => entries.body, + Err(ClientError::HttpError { status, .. }) if status == StatusCode::NOT_FOUND => { + vec![] + } + Err(err) => return Err(err)?, + }; + + tracing::info!(count = ?rfd_files.len(), "Fetched repo files"); + + let mut rfd_numbers_on_default = vec![]; + + // Each RFD should exist at a path that looks like rfd/{number}/README.adoc + for entry in rfd_files { + let path_parts = entry.path.split('/').collect::>(); + + // There should always be exactly 2 parts "rfd" "{number}" + if path_parts.len() == 2 { + if let Ok(number) = path_parts[1].parse::() { + rfd_numbers_on_default.push(number); + } else { + tracing::warn!(?path_parts, "Failed to parse RFD number from file path"); + } + } else { + tracing::warn!(?path_parts, path = ?entry.path, "Found RFD file with an invalid path"); + } + } + + let max_rfd_number_on_default = rfd_numbers_on_default.into_iter().max().unwrap_or(0); + + let branches = self.branches().await?; + let max_rfd_number_on_branch = branches + .iter() + .filter_map(|location| location.branch.parse::().ok()) + .max() + .unwrap_or(0); + + let next_rfd_number: RfdNumber = + (max_rfd_number_on_default.max(max_rfd_number_on_branch) + 1).into(); + + Ok(GitHubNewRfdNumber { + number: next_rfd_number, + commit: default.commit.sha, + }) + } + + pub async fn branches(&self) -> Result, GitHubError> { + let branch_pattern = Regex::new(r#"^\d{4}$"#).unwrap(); + let responses = self + .client + .repos() + .list_all_branches(&self.owner, &self.repo, false) + .await?; + Ok(responses + .body + .into_iter() + .filter_map(|branch| { + if branch_pattern.is_match(&branch.name) || branch.name == self.default_branch { + Some(self.location(branch.name, branch.commit.sha)) + } else { + None + } + }) + .collect()) + } + + #[instrument(skip(self), fields(owner = self.owner, repo = self.repo))] + pub async fn create_branch( + &self, + number: &RfdNumber, + commit: &str, + ) -> Result { + let ref_ = format!("refs/heads/{}", number.as_number_string()); + + tracing::debug!(ref_, "Creating GitHub ref"); + + self.client + .git() + .create_ref( + &self.owner, + &self.repo, + &GitCreateRefRequest { + key: String::new(), + ref_, + sha: commit.to_string(), + }, + ) + .await?; + + Ok(self.location(number.as_number_string(), commit.to_string())) + } + pub fn location(&self, branch: String, commit: String) -> GitHubRfdLocation { GitHubRfdLocation { client: self.client.clone(), @@ -313,23 +428,38 @@ impl Debug for GitHubRfdLocation { } impl GitHubRfdLocation { + pub fn is_default(&self) -> bool { + self.branch == self.default_branch + } + pub async fn readme_path(&self, client: &Client, rfd_number: &RfdNumber) -> String { // Use the supplied RFD number to determine the location in the RFD repo to read from let dir = rfd_number.repo_path(); // Get the contents of the file let mut path = format!("{}/README.adoc", dir); - let response = self.fetch_content(&client, &path, &self.commit).await; - - if let Err(err) = response { - tracing::trace!( - ?err, - "Failed to find asciidoc README, falling back to markdown" - ); - path = format!("{}/README.md", dir); + match self.fetch_content(&client, &path, &self.commit).await { + Ok(_) => path, + Err(err) => { + tracing::trace!( + ?err, + "Failed to find asciidoc README, falling back to markdown" + ); + + path = format!("{}/README.md", dir); + match self.fetch_content(&client, &path, &self.commit).await { + Ok(_) => path, + Err(err) => { + tracing::trace!( + ?err, + "Failed to find markdown README. With no README detected, defaulting to asciidoc" + ); + + format!("{}/README.adoc", dir) + } + } + } } - - path } /// Checks if this branch actually exists in the remote system (GitHub) @@ -548,9 +678,24 @@ impl GitHubRfdLocation { message: &str, ) -> Result<(), GitHubError> { let readme_path = self.readme_path(&self.client, rfd_number).await; - let FetchedRfdContent { decoded, sha, .. } = self + // let FetchedRfdContent { decoded, .. } = self + // .fetch_content(&self.client, &readme_path, &self.commit) + // .await?; + + let decoded = match self .fetch_content(&self.client, &readme_path, &self.commit) - .await?; + .await + { + Ok(FetchedRfdContent { decoded, .. }) => decoded, + Err(err) => match err { + GitHubError::ClientError(ClientError::HttpError { status, .. }) + if status == StatusCode::NOT_FOUND => + { + vec![] + } + other => return Err(other), + }, + }; // We can short circuit if the new and old content are the same if content == &decoded { @@ -572,7 +717,7 @@ impl GitHubRfdLocation { &readme_path.trim_start_matches('/'), &ReposCreateUpdateFileContentsRequest { message: format!("{}\nCommitted via rfd-api", message), - sha, + sha: self.commit.clone(), branch: self.branch.clone(), content: BASE64_STANDARD.encode(content), committer: Default::default(), @@ -614,6 +759,12 @@ pub struct GitHubRfdUpdate { pub committed_at: DateTime, } +#[derive(Debug, Clone)] +pub struct GitHubNewRfdNumber { + pub number: RfdNumber, + pub commit: String, +} + // TODO: Expand this pub fn is_image(file: &str) -> bool { file.ends_with(".svg") diff --git a/rfd-processor/src/content/mod.rs b/rfd-processor/src/content/mod.rs index e8120372..e46e2613 100644 --- a/rfd-processor/src/content/mod.rs +++ b/rfd-processor/src/content/mod.rs @@ -221,6 +221,10 @@ impl<'a> RfdDocument for RenderableRfd<'a> { RfdDocument::body(&self.content) } + fn update_body(&mut self, value: &str) { + RfdDocument::update_body(&mut self.content, value) + } + /// Get a reference to the internal unparsed contents fn raw(&self) -> &str { RfdDocument::raw(&self.content) diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index 511d137b..38c17bed 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -431,6 +431,7 @@ pub mod types { /// "ManageMappersAll", /// "GetRfdsAssigned", /// "GetRfdsAll", + /// "CreateRfd", /// "UpdateRfdsAssigned", /// "UpdateRfdsAll", /// "ManageRfdsVisibilityAssigned", @@ -961,6 +962,7 @@ pub mod types { ManageMappersAll, GetRfdsAssigned, GetRfdsAll, + CreateRfd, UpdateRfdsAssigned, UpdateRfdsAll, ManageRfdsVisibilityAssigned, @@ -1917,6 +1919,22 @@ pub mod types { /// } + /// }, + /// { + /// "type": "object", + /// "required": [ + /// "kind" + /// ], + /// "properties": { + /// "kind": { + /// "type": "string", + /// "enum": [ + /// "CreateRfd" + /// ] + /// } + + /// } + /// }, /// { /// "type": "object", @@ -2508,6 +2526,7 @@ pub mod types { GetRfds(Vec), GetRfdsAssigned, GetRfdsAll, + CreateRfd, UpdateRfd(i32), UpdateRfds(Vec), UpdateRfdsAssigned, @@ -4812,6 +4831,83 @@ pub mod types { } } + /// ReserveRfdBody + /// + ///
JSON schema + /// + /// ```json + /// { + /// "type": "object", + /// "required": [ + /// "title" + /// ], + /// "properties": { + /// "title": { + /// "type": "string" + /// } + + /// } + + /// } + + /// ``` + ///
+ #[derive(Clone, Debug, Deserialize, Serialize, schemars :: JsonSchema)] + pub struct ReserveRfdBody { + pub title: String, + } + + impl From<&ReserveRfdBody> for ReserveRfdBody { + fn from(value: &ReserveRfdBody) -> Self { + value.clone() + } + } + + impl ReserveRfdBody { + pub fn builder() -> builder::ReserveRfdBody { + Default::default() + } + } + + /// ReserveRfdResponse + /// + ///
JSON schema + /// + /// ```json + /// { + /// "type": "object", + /// "required": [ + /// "number" + /// ], + /// "properties": { + /// "number": { + /// "type": "integer", + /// "format": "int32" + /// } + + /// } + + /// } + + /// ``` + ///
+ #[derive(Clone, Debug, Deserialize, Serialize, schemars :: JsonSchema)] + pub struct ReserveRfdResponse { + pub number: i32, + } + + impl From<&ReserveRfdResponse> for ReserveRfdResponse { + fn from(value: &ReserveRfdResponse) -> Self { + value.clone() + } + } + + impl ReserveRfdResponse { + pub fn builder() -> builder::ReserveRfdResponse { + Default::default() + } + } + /// Rfd /// ///
JSON schema @@ -5214,11 +5310,11 @@ pub mod types { /// { /// "type": "object", /// "required": [ - /// "content" + /// "document" /// ], /// "properties": { - /// "content": { - /// "description": "Full Asciidoc content to store for this RFD", + /// "document": { + /// "description": "Full Asciidoc document to store for this RFD", /// "type": "string" /// }, /// "message": { @@ -5238,8 +5334,8 @@ pub mod types { ///
#[derive(Clone, Debug, Deserialize, Serialize, schemars :: JsonSchema)] pub struct RfdUpdateBody { - /// Full Asciidoc content to store for this RFD - pub content: String, + /// Full Asciidoc document to store for this RFD + pub document: String, /// Optional Git commit message to send with this update (recommended) #[serde(default, skip_serializing_if = "Option::is_none")] pub message: Option, @@ -5257,6 +5353,57 @@ pub mod types { } } + /// RfdUpdateContentBody + /// + ///
JSON schema + /// + /// ```json + /// { + /// "type": "object", + /// "required": [ + /// "content" + /// ], + /// "properties": { + /// "content": { + /// "description": "Asciidoc content to store for this RFD", + /// "type": "string" + /// }, + /// "message": { + /// "description": "Optional Git commit message to send with this + /// update (recommended)", + /// "type": [ + /// "string", + /// "null" + /// ] + /// } + + /// } + + /// } + + /// ``` + ///
+ #[derive(Clone, Debug, Deserialize, Serialize, schemars :: JsonSchema)] + pub struct RfdUpdateContentBody { + /// Asciidoc content to store for this RFD + pub content: String, + /// Optional Git commit message to send with this update (recommended) + #[serde(default, skip_serializing_if = "Option::is_none")] + pub message: Option, + } + + impl From<&RfdUpdateContentBody> for RfdUpdateContentBody { + fn from(value: &RfdUpdateContentBody) -> Self { + value.clone() + } + } + + impl RfdUpdateContentBody { + pub fn builder() -> builder::RfdUpdateContentBody { + Default::default() + } + } + /// RfdVisibility /// ///
JSON schema @@ -8836,6 +8983,92 @@ pub mod types { } } + #[derive(Clone, Debug)] + pub struct ReserveRfdBody { + title: Result, + } + + impl Default for ReserveRfdBody { + fn default() -> Self { + Self { + title: Err("no value supplied for title".to_string()), + } + } + } + + impl ReserveRfdBody { + pub fn title(mut self, value: T) -> Self + where + T: std::convert::TryInto, + T::Error: std::fmt::Display, + { + self.title = value + .try_into() + .map_err(|e| format!("error converting supplied value for title: {}", e)); + self + } + } + + impl std::convert::TryFrom for super::ReserveRfdBody { + type Error = super::error::ConversionError; + fn try_from(value: ReserveRfdBody) -> Result { + Ok(Self { + title: value.title?, + }) + } + } + + impl From for ReserveRfdBody { + fn from(value: super::ReserveRfdBody) -> Self { + Self { + title: Ok(value.title), + } + } + } + + #[derive(Clone, Debug)] + pub struct ReserveRfdResponse { + number: Result, + } + + impl Default for ReserveRfdResponse { + fn default() -> Self { + Self { + number: Err("no value supplied for number".to_string()), + } + } + } + + impl ReserveRfdResponse { + pub fn number(mut self, value: T) -> Self + where + T: std::convert::TryInto, + T::Error: std::fmt::Display, + { + self.number = value + .try_into() + .map_err(|e| format!("error converting supplied value for number: {}", e)); + self + } + } + + impl std::convert::TryFrom for super::ReserveRfdResponse { + type Error = super::error::ConversionError; + fn try_from(value: ReserveRfdResponse) -> Result { + Ok(Self { + number: value.number?, + }) + } + } + + impl From for ReserveRfdResponse { + fn from(value: super::ReserveRfdResponse) -> Self { + Self { + number: Ok(value.number), + } + } + } + #[derive(Clone, Debug)] pub struct Rfd { created_at: Result, String>, @@ -9022,28 +9255,28 @@ pub mod types { #[derive(Clone, Debug)] pub struct RfdUpdateBody { - content: Result, + document: Result, message: Result, String>, } impl Default for RfdUpdateBody { fn default() -> Self { Self { - content: Err("no value supplied for content".to_string()), + document: Err("no value supplied for document".to_string()), message: Ok(Default::default()), } } } impl RfdUpdateBody { - pub fn content(mut self, value: T) -> Self + pub fn document(mut self, value: T) -> Self where T: std::convert::TryInto, T::Error: std::fmt::Display, { - self.content = value + self.document = value .try_into() - .map_err(|e| format!("error converting supplied value for content: {}", e)); + .map_err(|e| format!("error converting supplied value for document: {}", e)); self } pub fn message(mut self, value: T) -> Self @@ -9062,7 +9295,7 @@ pub mod types { type Error = super::error::ConversionError; fn try_from(value: RfdUpdateBody) -> Result { Ok(Self { - content: value.content?, + document: value.document?, message: value.message?, }) } @@ -9070,6 +9303,65 @@ pub mod types { impl From for RfdUpdateBody { fn from(value: super::RfdUpdateBody) -> Self { + Self { + document: Ok(value.document), + message: Ok(value.message), + } + } + } + + #[derive(Clone, Debug)] + pub struct RfdUpdateContentBody { + content: Result, + message: Result, String>, + } + + impl Default for RfdUpdateContentBody { + fn default() -> Self { + Self { + content: Err("no value supplied for content".to_string()), + message: Ok(Default::default()), + } + } + } + + impl RfdUpdateContentBody { + pub fn content(mut self, value: T) -> Self + where + T: std::convert::TryInto, + T::Error: std::fmt::Display, + { + self.content = value + .try_into() + .map_err(|e| format!("error converting supplied value for content: {}", e)); + self + } + pub fn message(mut self, value: T) -> Self + where + T: std::convert::TryInto>, + T::Error: std::fmt::Display, + { + self.message = value + .try_into() + .map_err(|e| format!("error converting supplied value for message: {}", e)); + self + } + } + + impl std::convert::TryFrom for super::RfdUpdateContentBody { + type Error = super::error::ConversionError; + fn try_from( + value: RfdUpdateContentBody, + ) -> Result { + Ok(Self { + content: value.content?, + message: value.message?, + }) + } + } + + impl From for RfdUpdateContentBody { + fn from(value: super::RfdUpdateContentBody) -> Self { Self { content: Ok(value.content), message: Ok(value.message), @@ -9855,6 +10147,20 @@ impl Client { builder::GetRfds::new(self) } + /// Create a new RFD + /// + /// Sends a `POST` request to `/rfd` + /// + /// ```ignore + /// let response = client.reserve_rfd() + /// .body(body) + /// .send() + /// .await; + /// ``` + pub fn reserve_rfd(&self) -> builder::ReserveRfd { + builder::ReserveRfd::new(self) + } + /// Get the latest representation of an RFD /// /// Sends a `GET` request to `/rfd/{number}` @@ -9877,14 +10183,14 @@ impl Client { /// - `number`: The RFD number (examples: 1 or 123) /// - `body` /// ```ignore - /// let response = client.set_rfd_content() + /// let response = client.set_rfd_document() /// .number(number) /// .body(body) /// .send() /// .await; /// ``` - pub fn set_rfd_content(&self) -> builder::SetRfdContent { - builder::SetRfdContent::new(self) + pub fn set_rfd_document(&self) -> builder::SetRfdDocument { + builder::SetRfdDocument::new(self) } /// Get an attribute of a given RFD @@ -9918,6 +10224,22 @@ impl Client { builder::SetRfdAttr::new(self) } + /// Sends a `POST` request to `/rfd/{number}/content` + /// + /// Arguments: + /// - `number`: The RFD number (examples: 1 or 123) + /// - `body` + /// ```ignore + /// let response = client.set_rfd_content() + /// .number(number) + /// .body(body) + /// .send() + /// .await; + /// ``` + pub fn set_rfd_content(&self) -> builder::SetRfdContent { + builder::SetRfdContent::new(self) + } + /// Modify the visibility of an RFD /// /// Sends a `POST` request to `/rfd/{number}/visibility` @@ -12349,6 +12671,77 @@ pub mod builder { } } + /// Builder for [`Client::reserve_rfd`] + /// + /// [`Client::reserve_rfd`]: super::Client::reserve_rfd + #[derive(Debug, Clone)] + pub struct ReserveRfd<'a> { + client: &'a super::Client, + body: Result, + } + + impl<'a> ReserveRfd<'a> { + pub fn new(client: &'a super::Client) -> Self { + Self { + client: client, + body: Ok(types::builder::ReserveRfdBody::default()), + } + } + + pub fn body(mut self, value: V) -> Self + where + V: std::convert::TryInto, + >::Error: std::fmt::Display, + { + self.body = value + .try_into() + .map(From::from) + .map_err(|s| format!("conversion to `ReserveRfdBody` for body failed: {}", s)); + self + } + + pub fn body_map(mut self, f: F) -> Self + where + F: std::ops::FnOnce(types::builder::ReserveRfdBody) -> types::builder::ReserveRfdBody, + { + self.body = self.body.map(f); + self + } + + /// Sends a `POST` request to `/rfd` + pub async fn send( + self, + ) -> Result, Error> { + let Self { client, body } = self; + let body = body + .and_then(|v| types::ReserveRfdBody::try_from(v).map_err(|e| e.to_string())) + .map_err(Error::InvalidRequest)?; + let url = format!("{}/rfd", client.baseurl,); + #[allow(unused_mut)] + let mut request = client + .client + .post(url) + .header( + reqwest::header::ACCEPT, + reqwest::header::HeaderValue::from_static("application/json"), + ) + .json(&body) + .build()?; + let result = client.client.execute(request).await; + let response = result?; + match response.status().as_u16() { + 202u16 => ResponseValue::from_response(response).await, + 400u16..=499u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + 500u16..=599u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + _ => Err(Error::UnexpectedResponse(response)), + } + } + } + /// Builder for [`Client::get_rfd`] /// /// [`Client::get_rfd`]: super::Client::get_rfd @@ -12409,17 +12802,17 @@ pub mod builder { } } - /// Builder for [`Client::set_rfd_content`] + /// Builder for [`Client::set_rfd_document`] /// - /// [`Client::set_rfd_content`]: super::Client::set_rfd_content + /// [`Client::set_rfd_document`]: super::Client::set_rfd_document #[derive(Debug, Clone)] - pub struct SetRfdContent<'a> { + pub struct SetRfdDocument<'a> { client: &'a super::Client, number: Result, body: Result, } - impl<'a> SetRfdContent<'a> { + impl<'a> SetRfdDocument<'a> { pub fn new(client: &'a super::Client) -> Self { Self { client: client, @@ -12670,7 +13063,101 @@ pub mod builder { let result = client.client.execute(request).await; let response = result?; match response.status().as_u16() { - 200u16 => ResponseValue::from_response(response).await, + 202u16 => ResponseValue::from_response(response).await, + 400u16..=499u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + 500u16..=599u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + _ => Err(Error::UnexpectedResponse(response)), + } + } + } + + /// Builder for [`Client::set_rfd_content`] + /// + /// [`Client::set_rfd_content`]: super::Client::set_rfd_content + #[derive(Debug, Clone)] + pub struct SetRfdContent<'a> { + client: &'a super::Client, + number: Result, + body: Result, + } + + impl<'a> SetRfdContent<'a> { + pub fn new(client: &'a super::Client) -> Self { + Self { + client: client, + number: Err("number was not initialized".to_string()), + body: Ok(types::builder::RfdUpdateContentBody::default()), + } + } + + pub fn number(mut self, value: V) -> Self + where + V: std::convert::TryInto, + { + self.number = value + .try_into() + .map_err(|_| "conversion to `String` for number failed".to_string()); + self + } + + pub fn body(mut self, value: V) -> Self + where + V: std::convert::TryInto, + >::Error: std::fmt::Display, + { + self.body = value.try_into().map(From::from).map_err(|s| { + format!( + "conversion to `RfdUpdateContentBody` for body failed: {}", + s + ) + }); + self + } + + pub fn body_map(mut self, f: F) -> Self + where + F: std::ops::FnOnce( + types::builder::RfdUpdateContentBody, + ) -> types::builder::RfdUpdateContentBody, + { + self.body = self.body.map(f); + self + } + + /// Sends a `POST` request to `/rfd/{number}/content` + pub async fn send(self) -> Result, Error> { + let Self { + client, + number, + body, + } = self; + let number = number.map_err(Error::InvalidRequest)?; + let body = body + .and_then(|v| types::RfdUpdateContentBody::try_from(v).map_err(|e| e.to_string())) + .map_err(Error::InvalidRequest)?; + let url = format!( + "{}/rfd/{}/content", + client.baseurl, + encode_path(&number.to_string()), + ); + #[allow(unused_mut)] + let mut request = client + .client + .post(url) + .header( + reqwest::header::ACCEPT, + reqwest::header::HeaderValue::from_static("application/json"), + ) + .json(&body) + .build()?; + let result = client.client.execute(request).await; + let response = result?; + match response.status().as_u16() { + 202u16 => ResponseValue::from_response(response).await, 400u16..=499u16 => Err(Error::ErrorResponse( ResponseValue::from_response(response).await?, )), diff --git a/rfd-sdk/src/lib.rs b/rfd-sdk/src/lib.rs index f177ebbf..1843ebc2 100644 --- a/rfd-sdk/src/lib.rs +++ b/rfd-sdk/src/lib.rs @@ -91,6 +91,7 @@ impl Display for ApiPermissionResponse { ), Self::GetRfdsAssigned => write!(f, "get-rfds-assigned"), Self::GetRfdsAll => write!(f, "get-rfds-all"), + Self::CreateRfd => write!(f, "create-rfd"), Self::UpdateRfd(number) => write!(f, "update-rfd:{}", number), Self::UpdateRfds(numbers) => write!( f, From e29d09eb438608b1b5538b2b56c55e6667b67498 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 19 Apr 2024 11:30:02 -0500 Subject: [PATCH 02/26] Content update fixes. CLI output for RFD reservation --- rfd-api-spec.json | 6 ++++ rfd-api/src/context.rs | 48 +++++++++++++++++++++++--------- rfd-api/src/endpoints/rfd.rs | 5 +++- rfd-cli/src/generated/cli.rs | 14 +++++++++- rfd-cli/src/main.rs | 4 +++ rfd-cli/src/printer/json.rs | 4 +++ rfd-cli/src/printer/mod.rs | 8 ++++++ rfd-cli/src/printer/tab.rs | 14 ++++++++-- rfd-data/src/content/asciidoc.rs | 48 ++++++++++++++++++++++++-------- rfd-data/src/content/template.rs | 2 +- rfd-sdk/src/generated/sdk.rs | 26 +++++++++++++++++ 11 files changed, 149 insertions(+), 30 deletions(-) diff --git a/rfd-api-spec.json b/rfd-api-spec.json index 9e18f226..4ec8d160 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -4568,7 +4568,13 @@ "ReserveRfdBody": { "type": "object", "properties": { + "content": { + "nullable": true, + "description": "Optional contents of the RFD", + "type": "string" + }, "title": { + "description": "Title of the RFD", "type": "string" } }, diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index d026cef1..444d9485 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -153,7 +153,8 @@ pub struct SearchContext { } pub struct ContentContext { - pub new_rfd_template: RfdTemplate, + pub placeholder_template: RfdTemplate, + pub new_template: RfdTemplate, } pub struct RegisteredAccessToken { @@ -322,7 +323,12 @@ impl ApiContext { client: SearchClient::new(search.host, search.index, search.key), }, content: ContentContext { - new_rfd_template: content + placeholder_template: content + .templates + .get("placeholder") + .cloned() + .ok_or(AppError::MissingNewRfdTemplate)?, + new_template: content .templates .get("new") .cloned() @@ -692,6 +698,7 @@ impl ApiContext { &self, caller: &ApiCaller, title: String, + content: Option, ) -> ResourceResult { if caller.can(&ApiPermission::CreateRfd) { tracing::info!("Reserving new RFD"); @@ -708,6 +715,25 @@ impl ApiContext { .map_err(UpdateRfdContentError::GitHub) .to_resource_result()?; + let content = match content { + Some(content) => self + .content + .new_template + .clone() + .field("number".to_string(), next_rfd_number.to_string()) + .field("title".to_string(), title) + .field("body".to_string(), content), + None => self + .content + .placeholder_template + .clone() + .field("number".to_string(), next_rfd_number.to_string()) + .field("title".to_string(), title), + } + .build() + .map_err(UpdateRfdContentError::InvalidTemplate) + .to_resource_result()?; + tracing::info!(?next_rfd_number, commit, "Creating new RFD branch"); // Branch off of the default branch with a new branch with the padded form of the RFD number @@ -723,16 +749,6 @@ impl ApiContext { "Created new branch for reserving RFD off of default branch" ); - let content = self - .content - .new_rfd_template - .clone() - .field("number".to_string(), next_rfd_number.to_string()) - .field("title".to_string(), title) - .build() - .map_err(UpdateRfdContentError::InvalidTemplate) - .to_resource_result()?; - self.commit_rfd_document( caller, next_rfd_number.into(), @@ -2224,6 +2240,7 @@ mod tests { #[cfg(test)] pub(crate) mod test_mocks { use async_trait::async_trait; + use rfd_data::content::RfdTemplate; use rfd_model::{ storage::{ AccessGroupStore, AccessTokenStore, ApiKeyStore, ApiUserProviderStore, ApiUserStore, @@ -2254,6 +2271,11 @@ pub(crate) mod test_mocks { // Construct a mock context that can be used in tests pub async fn mock_context(storage: MockStorage) -> ApiContext { + let mut content = ContentConfig::default(); + content + .templates + .insert("new".to_string(), RfdTemplate::default()); + let mut ctx = ApiContext::new( "".to_string(), Arc::new(storage), @@ -2263,7 +2285,7 @@ pub(crate) mod test_mocks { mock_key(), ], SearchConfig::default(), - ContentConfig::default(), + content, ServicesConfig { github: GitHubConfig { owner: String::new(), diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index f84133f6..222718b0 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -55,7 +55,10 @@ async fn get_rfds_op( #[derive(Debug, Deserialize, JsonSchema)] pub struct ReserveRfdBody { + /// Title of the RFD pub title: String, + /// Optional contents of the RFD + pub content: Option, } #[derive(Debug, Deserialize, Serialize, JsonSchema)] @@ -90,7 +93,7 @@ async fn reserve_rfd_op( caller: &ApiCaller, body: ReserveRfdBody, ) -> Result, HttpError> { - let number = ctx.create_rfd(caller, body.title).await?; + let number = ctx.create_rfd(caller, body.title, body.content).await?; Ok(HttpResponseAccepted(ReserveRfdResponse { number: number.into(), })) diff --git a/rfd-cli/src/generated/cli.rs b/rfd-cli/src/generated/cli.rs index b843bdee..a18da517 100644 --- a/rfd-cli/src/generated/cli.rs +++ b/rfd-cli/src/generated/cli.rs @@ -698,11 +698,19 @@ impl Cli { pub fn cli_reserve_rfd() -> clap::Command { clap::Command::new("") + .arg( + clap::Arg::new("content") + .long("content") + .value_parser(clap::value_parser!(String)) + .required(false) + .help("Optional contents of the RFD"), + ) .arg( clap::Arg::new("title") .long("title") .value_parser(clap::value_parser!(String)) - .required_unless_present("json-body"), + .required_unless_present("json-body") + .help("Title of the RFD"), ) .arg( clap::Arg::new("json-body") @@ -1889,6 +1897,10 @@ impl Cli { pub async fn execute_reserve_rfd(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { let mut request = self.client.reserve_rfd(); + if let Some(value) = matches.get_one::("content") { + request = request.body_map(|body| body.content(value.clone())) + } + if let Some(value) = matches.get_one::("title") { request = request.body_map(|body| body.title(value.clone())) } diff --git a/rfd-cli/src/main.rs b/rfd-cli/src/main.rs index 0363581c..bb2f3cb7 100644 --- a/rfd-cli/src/main.rs +++ b/rfd-cli/src/main.rs @@ -371,6 +371,10 @@ impl ProgenitorCliConfig for Context { .unwrap() .output_search_results(reserialize(value)), "RfdAttr" => self.printer().unwrap().output_rfd_attr(reserialize(value)), + "ReserveRfdResponse" => self + .printer() + .unwrap() + .output_reserved_rfd(reserialize(value)), other => eprintln!( "Unhandled response type: {}. Please report this as a bug.", other diff --git a/rfd-cli/src/printer/json.rs b/rfd-cli/src/printer/json.rs index e00346f5..9f86d777 100644 --- a/rfd-cli/src/printer/json.rs +++ b/rfd-cli/src/printer/json.rs @@ -87,6 +87,10 @@ impl CliOutput for RfdJsonPrinter { println!("{}", serde_json::to_string(&value).unwrap()) } + fn output_reserved_rfd(&self, value: types::ReserveRfdResponse) { + println!("{}", serde_json::to_string(&value).unwrap()) + } + fn output_error(&self, value: &progenitor_client::Error) where T: schemars::JsonSchema + serde::Serialize + std::fmt::Debug, diff --git a/rfd-cli/src/printer/mod.rs b/rfd-cli/src/printer/mod.rs index 6f1eb955..01bdbef6 100644 --- a/rfd-cli/src/printer/mod.rs +++ b/rfd-cli/src/printer/mod.rs @@ -35,6 +35,7 @@ pub trait CliOutput { fn output_rfd(&self, value: types::Rfd) {} fn output_rfd_attr(&self, value: types::RfdAttr) {} fn output_search_results(&self, value: types::SearchResults) {} + fn output_reserved_rfd(&self, value: types::ReserveRfdResponse) {} fn output_error(&self, value: &progenitor_client::Error) where T: schemars::JsonSchema + serde::Serialize + std::fmt::Debug; @@ -174,6 +175,13 @@ impl CliOutput for Printer { } } + fn output_reserved_rfd(&self, value: types::ReserveRfdResponse) { + match self { + Self::Json(printer) => printer.output_reserved_rfd(value), + Self::Tab(printer) => printer.output_reserved_rfd(value), + } + } + fn output_error(&self, value: &progenitor_client::Error) where T: schemars::JsonSchema + serde::Serialize + std::fmt::Debug, diff --git a/rfd-cli/src/printer/tab.rs b/rfd-cli/src/printer/tab.rs index ad2ff57a..98de7d2b 100644 --- a/rfd-cli/src/printer/tab.rs +++ b/rfd-cli/src/printer/tab.rs @@ -9,8 +9,8 @@ use rfd_sdk::types::{ self, AccessGroupForApiPermissionResponse, ApiKeyResponse, ApiPermission, ApiUserForApiPermissionResponse, Error, FullRfd, FullRfdPdfEntry, GetUserResponse, InitialApiKeyResponse, InitialOAuthClientSecretResponse, ListRfd, Mapper, OAuthClient, - OAuthClientRedirectUri, OAuthClientSecret, PermissionsForApiPermissionResponse, RfdAttr, - SearchResultHit, SearchResults, Visibility, + OAuthClientRedirectUri, OAuthClientSecret, PermissionsForApiPermissionResponse, + ReserveRfdResponse, RfdAttr, SearchResultHit, SearchResults, Visibility, }; use std::{collections::HashMap, fmt::Display, fs::File, io::Write, process::Command}; use tabwriter::TabWriter; @@ -141,6 +141,10 @@ impl CliOutput for RfdTabPrinter { self.print_cli_output(&value, Some("results".to_string())); } + fn output_reserved_rfd(&self, value: types::ReserveRfdResponse) { + self.print_cli_output(&value, None); + } + fn output_error(&self, value: &progenitor_client::Error) where T: schemars::JsonSchema + serde::Serialize + std::fmt::Debug, @@ -560,6 +564,12 @@ impl TabDisplay for SearchResultHit { } } +impl TabDisplay for ReserveRfdResponse { + fn display(&self, tw: &mut TabWriter>, level: u8, printer: &RfdTabPrinter) { + printer.print_field(tw, level, "number", &self.number); + } +} + impl TabDisplay for Vec where T: TabDisplay, diff --git a/rfd-data/src/content/asciidoc.rs b/rfd-data/src/content/asciidoc.rs index 9f2b62c8..bba2b6aa 100644 --- a/rfd-data/src/content/asciidoc.rs +++ b/rfd-data/src/content/asciidoc.rs @@ -41,7 +41,7 @@ impl<'a> RfdAsciidoc<'a> { .replacen(found.as_str(), &format!(":{}: {}\n", attr, value), 1) .into() } else { - let title = self.title_pattern().find(&self.content); + let title = self.title_line(); if let Some(title) = title { let new_attr = format!(":{}: {}\n", attr, value); @@ -51,7 +51,8 @@ impl<'a> RfdAsciidoc<'a> { self.content = (header.unwrap_or_default().to_string() + "\n" + &new_attr - + title.as_str() + + "\n\n" + + title + body.unwrap_or_default()) .into() } @@ -62,6 +63,11 @@ impl<'a> RfdAsciidoc<'a> { Regex::new(&format!(r"(?m)^:{}:(.*)$\n", attr)).unwrap() } + fn title_line(&self) -> Option<&str> { + let title = self.title_pattern().find(&self.content); + title.map(|m| m.as_str()) + } + fn title_pattern(&self) -> Regex { // This pattern also include markdown title handling fallbacks to handle malformed // documents @@ -151,14 +157,12 @@ impl<'a> RfdDocument for RfdAsciidoc<'a> { } fn update_body(&mut self, value: &str) { - self.content = Cow::Owned( - self.title_pattern() - .splitn(&self.content, 2) - .nth(0) - .unwrap_or_default() - .to_string() - + value, - ) + self.content = Cow::Owned(format!( + "{}\n\n{}{}", + self.header().unwrap_or_default(), + self.title_line().unwrap_or_default(), + value + )); } /// Get a reference to the internal unparsed contents @@ -451,8 +455,7 @@ sdf } fn test_rfd_content() -> &'static str { - r#" -:reproducible: + r#":reproducible: :showtitle: :toc: left :numbered: @@ -518,4 +521,25 @@ in velit. let expected = "newlabel1, newlabel2".to_string(); assert_eq!(expected, labels); } + + #[test] + fn test_update_body() { + let new_content = r#"This is the new body"#; + let expected = r#":reproducible: +:showtitle: +:toc: left +:numbered: +:icons: font +:state: prediscussion +:revremark: State: {state} +:docdatetime: 2019-01-04 19:26:06 UTC +:localdatetime: 2019-01-04 19:26:06 UTC +:labels: label1, label2 + += RFD 123 Place +This is the new body"#; + let mut rfd = RfdAsciidoc::new(Cow::Borrowed(test_rfd_content())); + rfd.update_body(&new_content); + assert_eq!(expected, rfd.raw()); + } } diff --git a/rfd-data/src/content/template.rs b/rfd-data/src/content/template.rs index 2a6d508f..74f70c14 100644 --- a/rfd-data/src/content/template.rs +++ b/rfd-data/src/content/template.rs @@ -11,7 +11,7 @@ pub enum TemplateError { }, } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Deserialize, Serialize, Default)] pub struct RfdTemplate { template: String, #[serde(default)] diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index 38c17bed..61a33d54 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -4842,7 +4842,15 @@ pub mod types { /// "title" /// ], /// "properties": { + /// "content": { + /// "description": "Optional contents of the RFD", + /// "type": [ + /// "string", + /// "null" + /// ] + /// }, /// "title": { + /// "description": "Title of the RFD", /// "type": "string" /// } @@ -4854,6 +4862,10 @@ pub mod types { ///
#[derive(Clone, Debug, Deserialize, Serialize, schemars :: JsonSchema)] pub struct ReserveRfdBody { + /// Optional contents of the RFD + #[serde(default, skip_serializing_if = "Option::is_none")] + pub content: Option, + /// Title of the RFD pub title: String, } @@ -8985,18 +8997,30 @@ pub mod types { #[derive(Clone, Debug)] pub struct ReserveRfdBody { + content: Result, String>, title: Result, } impl Default for ReserveRfdBody { fn default() -> Self { Self { + content: Ok(Default::default()), title: Err("no value supplied for title".to_string()), } } } impl ReserveRfdBody { + pub fn content(mut self, value: T) -> Self + where + T: std::convert::TryInto>, + T::Error: std::fmt::Display, + { + self.content = value + .try_into() + .map_err(|e| format!("error converting supplied value for content: {}", e)); + self + } pub fn title(mut self, value: T) -> Self where T: std::convert::TryInto, @@ -9013,6 +9037,7 @@ pub mod types { type Error = super::error::ConversionError; fn try_from(value: ReserveRfdBody) -> Result { Ok(Self { + content: value.content?, title: value.title?, }) } @@ -9021,6 +9046,7 @@ pub mod types { impl From for ReserveRfdBody { fn from(value: super::ReserveRfdBody) -> Self { Self { + content: Ok(value.content), title: Ok(value.title), } } From bf5afc39353cd72785cf6ce34636c04d85c7629f Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 19 Apr 2024 12:22:17 -0500 Subject: [PATCH 03/26] Extend conenction timeout limit --- rfd-model/src/storage/postgres.rs | 2 +- rfd-processor/src/processor.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 5e85f0f8..805fd421 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -77,7 +77,7 @@ impl PostgresStore { Ok(Self { pool: Pool::builder() - .connection_timeout(Duration::from_secs(5)) + .connection_timeout(Duration::from_secs(15)) .build(manager) .await?, }) diff --git a/rfd-processor/src/processor.rs b/rfd-processor/src/processor.rs index 458e92a3..3cb0e5cc 100644 --- a/rfd-processor/src/processor.rs +++ b/rfd-processor/src/processor.rs @@ -42,7 +42,7 @@ pub async fn processor(ctx: Arc) -> Result<(), JobError> { let ctx = ctx.clone(); tokio::spawn(async move { - // Make the job as started + // Mark the job as started match JobStore::start(&ctx.db.storage, job.id).await { Ok(Some(job)) => { let location = GitHubRfdLocation { From 981ad06159e48d6bbba6ad28f4e8878d1d889b9c Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 19 Apr 2024 12:36:42 -0500 Subject: [PATCH 04/26] Add index --- .../migrations/2024-04-19-173349_add_jobs_started_index/down.sql | 1 + .../migrations/2024-04-19-173349_add_jobs_started_index/up.sql | 1 + 2 files changed, 2 insertions(+) create mode 100644 rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/down.sql create mode 100644 rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/up.sql diff --git a/rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/down.sql b/rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/down.sql new file mode 100644 index 00000000..cd074b67 --- /dev/null +++ b/rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/down.sql @@ -0,0 +1 @@ +DROP INDEX jobs_started; \ No newline at end of file diff --git a/rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/up.sql b/rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/up.sql new file mode 100644 index 00000000..93658ac7 --- /dev/null +++ b/rfd-model/migrations/2024-04-19-173349_add_jobs_started_index/up.sql @@ -0,0 +1 @@ +CREATE INDEX jobs_started ON job (id, started_at, processed ASC, committed_at ASC, created_at ASC); \ No newline at end of file From 6bdeee5dd0aba19b18a2b418d0eb36f587f5f5f6 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 19 Apr 2024 14:48:05 -0500 Subject: [PATCH 05/26] More cli help messages --- rfd-api-spec.json | 2 ++ rfd-api/src/endpoints/rfd.rs | 2 ++ rfd-cli/src/generated/cli.rs | 2 ++ rfd-sdk/src/generated/sdk.rs | 4 ++++ 4 files changed, 10 insertions(+) diff --git a/rfd-api-spec.json b/rfd-api-spec.json index 4ec8d160..dd09d33a 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -1285,6 +1285,7 @@ } }, "post": { + "summary": "Replace the full document of for a RFD", "operationId": "set_rfd_document", "parameters": [ { @@ -1425,6 +1426,7 @@ }, "/rfd/{number}/content": { "post": { + "summary": "Replace the contents of an RFD", "operationId": "set_rfd_content", "parameters": [ { diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 222718b0..8f90d5d9 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -150,6 +150,7 @@ pub struct RfdUpdateBody { message: Option, } +/// Replace the full document of for a RFD #[trace_request] #[endpoint { method = POST, @@ -203,6 +204,7 @@ pub struct RfdUpdateContentBody { message: Option, } +/// Replace the contents of an RFD #[trace_request] #[endpoint { method = POST, diff --git a/rfd-cli/src/generated/cli.rs b/rfd-cli/src/generated/cli.rs index a18da517..73d84cfa 100644 --- a/rfd-cli/src/generated/cli.rs +++ b/rfd-cli/src/generated/cli.rs @@ -778,6 +778,7 @@ impl Cli { .action(clap::ArgAction::SetTrue) .help("XXX"), ) + .about("Replace the full document of for a RFD") } pub fn cli_get_rfd_attr() -> clap::Command { @@ -893,6 +894,7 @@ impl Cli { .action(clap::ArgAction::SetTrue) .help("XXX"), ) + .about("Replace the contents of an RFD") } pub fn cli_update_rfd_visibility() -> clap::Command { diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index 61a33d54..67f510f3 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -10203,6 +10203,8 @@ impl Client { builder::GetRfd::new(self) } + /// Replace the full document of for a RFD + /// /// Sends a `POST` request to `/rfd/{number}` /// /// Arguments: @@ -10250,6 +10252,8 @@ impl Client { builder::SetRfdAttr::new(self) } + /// Replace the contents of an RFD + /// /// Sends a `POST` request to `/rfd/{number}/content` /// /// Arguments: From 9a12fb61ae2de974c11cc13f8f6fe6f9428cd59b Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 22 Apr 2024 10:29:53 -0500 Subject: [PATCH 06/26] Add discuss and publish endpoints --- README.md | 12 +- rfd-api-spec.json | 82 ++++++++- rfd-api/src/context.rs | 4 +- rfd-api/src/endpoints/rfd.rs | 66 ++++++- rfd-api/src/endpoints/webhook.rs | 2 +- rfd-api/src/server.rs | 7 +- rfd-cli/src/generated/cli.rs | 98 +++++++++- rfd-cli/src/main.rs | 2 + rfd-data/src/lib.rs | 2 +- .../src/updater/ensure_default_state.rs | 2 +- rfd-processor/src/updater/mod.rs | 4 +- rfd-processor/src/updater/update_pdfs.rs | 2 +- rfd-sdk/src/generated/sdk.rs | 168 +++++++++++++++++- 13 files changed, 407 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index e801fa40..d34868a8 100644 --- a/README.md +++ b/README.md @@ -95,7 +95,7 @@ determining the RFD number to update. Instead they use the numeric portion of th ### Revisions -Every revision is tied to a commit* against an RFD readme file. There is no guarantee though that +Every revision is tied to a commit* against a RFD readme file. There is no guarantee though that there exists a revision though for every commit. While the RFD API will attempt to create a revision for every commit, outages, missing webhooks, or internal errors can result in missing revisions. Currently the background periodic processor does not attempt to backfill missing revisions, it only @@ -111,7 +111,7 @@ as a separate action. Currently the supported actions are: | Action | Purpose | |------------------------|------------ -| copy_images_to_storage | Copies images and static files associated with an RFD to cloud storage +| copy_images_to_storage | Copies images and static files associated with a RFD to cloud storage | create_pull_request | Create a PR for the RFD if it does not have one and the RFD is in discussion | ensure_default_state | Checks that RFDs on the default branch have appropriate states | ensure_pr_state | Updates the state attribute for RFDs not on the default branch as needed @@ -123,16 +123,16 @@ as a separate action. Currently the supported actions are: ### Content Updates RFD processing manipulates both internally stored state as well as the source content document of -the RFD it is processing. The two cases where the processor will update the contents of an RFD are: +the RFD it is processing. The two cases where the processor will update the contents of a RFD are: -1. An RFD has an incorrect discussion url -2. An RFD is in an incorrect state +1. a RFD has an incorrect discussion url +2. a RFD is in an incorrect state The first update is the easier of the two. For any RFD that has an open discussion PR, the processor will check that the `discussion` attribute in the RFD document matches the url of the discussion PR. Note though that there is a bug here currently related to the order in which revisions may be processed. -State checking is a bit more complex. For an RFD that has an open discussion PR, the processor will +State checking is a bit more complex. For a RFD that has an open discussion PR, the processor will ensure that the RFD state is set to `discussion`. For RFDs that are merged to the default branch though, there is not a good determination as to which of the final states to assign them. Instead the processor will emit a warning when it encounters such a case. diff --git a/rfd-api-spec.json b/rfd-api-spec.json index dd09d33a..84ff9b22 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -1252,7 +1252,7 @@ }, "/rfd/{number}": { "get": { - "summary": "Get the latest representation of an RFD", + "summary": "Get the latest representation of a RFD", "operationId": "get_rfd", "parameters": [ { @@ -1334,7 +1334,7 @@ }, "/rfd/{number}/attr/{attr}": { "get": { - "summary": "Get an attribute of a given RFD", + "summary": "Get an attribute of a RFD", "operationId": "get_rfd_attr", "parameters": [ { @@ -1373,8 +1373,8 @@ } } }, - "put": { - "summary": "Set an attribute of a given RFD", + "post": { + "summary": "Set an attribute of a RFD", "operationId": "set_rfd_attr", "parameters": [ { @@ -1426,7 +1426,7 @@ }, "/rfd/{number}/content": { "post": { - "summary": "Replace the contents of an RFD", + "summary": "Replace the contents of a RFD", "operationId": "set_rfd_content", "parameters": [ { @@ -1473,9 +1473,79 @@ } } }, + "/rfd/{number}/discuss": { + "post": { + "summary": "Open a RFD for discussion", + "operationId": "discuss_rfd", + "parameters": [ + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdAttr" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/rfd/{number}/publish": { + "post": { + "summary": "Publish a RFD", + "operationId": "publish_rfd", + "parameters": [ + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "202": { + "description": "successfully enqueued operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdAttr" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/rfd/{number}/visibility": { "post": { - "summary": "Modify the visibility of an RFD", + "summary": "Modify the visibility of a RFD", "operationId": "update_rfd_visibility", "parameters": [ { diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 444d9485..b232608c 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -704,7 +704,7 @@ impl ApiContext { tracing::info!("Reserving new RFD"); // We acknowledge that there are race conditions here, as there would be if an end user - // were to attempt to reserve an RFD number manually + // were to attempt to reserve a RFD number manually let GitHubNewRfdNumber { number: next_rfd_number, commit, @@ -787,7 +787,7 @@ impl ApiContext { .await?; if let Some(rfd) = rfds.into_iter().nth(0) { - // If list_rfds returned an RFD, then the caller is allowed to access that RFD and we + // If list_rfds returned a RFD, then the caller is allowed to access that RFD and we // can return the full RFD revision. This is sub-optimal as we are required to execute // the revision lookup twice let latest_revision = RfdRevisionStore::list( diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 8f90d5d9..5bfd4a5d 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -105,7 +105,7 @@ pub struct RfdPathParams { number: String, } -/// Get the latest representation of an RFD +/// Get the latest representation of a RFD #[trace_request] #[endpoint { method = GET, @@ -204,7 +204,7 @@ pub struct RfdUpdateContentBody { message: Option, } -/// Replace the contents of an RFD +/// Replace the contents of a RFD #[trace_request] #[endpoint { method = POST, @@ -272,7 +272,7 @@ pub enum RfdAttr { State(RfdState), } -/// Get an attribute of a given RFD +/// Get an attribute of a RFD #[trace_request] #[endpoint { method = GET, @@ -326,10 +326,10 @@ pub struct RfdAttrValue { message: Option, } -/// Set an attribute of a given RFD +/// Set an attribute of a RFD #[trace_request] #[endpoint { - method = PUT, + method = POST, path = "/rfd/{number}/attr/{attr}", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] @@ -403,6 +403,54 @@ async fn set_rfd_attr_op( } } +/// Open a RFD for discussion +#[trace_request] +#[endpoint { + method = POST, + path = "/rfd/{number}/discuss", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn discuss_rfd( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let auth = ctx.authn_token(&rqctx).await?; + let path = path.into_inner(); + set_rfd_attr_op( + ctx, + &ctx.get_caller(auth.as_ref()).await?, + path.number, + RfdAttrName::State, + &RfdAttrValue { value: RfdState::Discussion.to_string(), message: Some("Move to discussion".to_string()) }, + ) + .await +} + +/// Publish a RFD +#[trace_request] +#[endpoint { + method = POST, + path = "/rfd/{number}/publish", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn publish_rfd( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let auth = ctx.authn_token(&rqctx).await?; + let path = path.into_inner(); + set_rfd_attr_op( + ctx, + &ctx.get_caller(auth.as_ref()).await?, + path.number, + RfdAttrName::State, + &RfdAttrValue { value: RfdState::Published.to_string(), message: Some("Publish".to_string()) }, + ) + .await +} + fn extract_attr(attr: &RfdAttrName, content: &RfdContent) -> Result { match attr { RfdAttrName::Discussion => content @@ -582,7 +630,7 @@ pub struct RfdVisibility { pub visibility: Visibility, } -/// Modify the visibility of an RFD +/// Modify the visibility of a RFD #[trace_request] #[endpoint { method = POST, @@ -809,7 +857,7 @@ mod tests { assert_eq!(456, rfd.rfd_number); } - // Test RFD access via the direct permission to an RFD + // Test RFD access via the direct permission to a RFD #[tokio::test] async fn list_rfds_with_direct_permission() { @@ -868,7 +916,7 @@ mod tests { match result { Err(err) => assert_eq!(StatusCode::FORBIDDEN, err.status_code), Ok(response) => panic!( - "Expected a 403 error, but instead found an RFD {:?}", + "Expected a 403 error, but instead found a RFD {:?}", response.0 ), } @@ -899,7 +947,7 @@ mod tests { match result { Err(err) => assert_eq!(StatusCode::FORBIDDEN, err.status_code), Ok(response) => panic!( - "Expected a 403 error, but instead found an RFD {:?}", + "Expected a 403 error, but instead found a RFD {:?}", response.0 ), } diff --git a/rfd-api/src/endpoints/webhook.rs b/rfd-api/src/endpoints/webhook.rs index 867fd921..05dc8a0d 100644 --- a/rfd-api/src/endpoints/webhook.rs +++ b/rfd-api/src/endpoints/webhook.rs @@ -95,7 +95,7 @@ impl GitHubCommitPayload { fn affected_rfds(&self) -> Vec { // Check the committed files for changes to specific RFDs. Depending on the branch of the // commit, changes will be accepted to rejected. Changes on the default repository branch - // are accepted for all RFDs, but on an RFD specific branch (i.e. 0123) on changes to + // are accepted for all RFDs, but on a RFD specific branch (i.e. 0123) on changes to // RFD 123 are accepted. Changes on non-default, non-rfd branches are always rejected let pattern = Regex::new(r#"^rfd/(\d{4})/"#).unwrap(); let branch = self.branch(); diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index 20ef28ab..bb34ea57 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -30,8 +30,7 @@ use crate::{ }, mappers::{create_mapper, delete_mapper, get_mappers}, rfd::{ - get_rfd, get_rfd_attr, get_rfds, reserve_rfd, search_rfds, set_rfd_attr, - set_rfd_content, set_rfd_document, update_rfd_visibility, + discuss_rfd, get_rfd, get_rfd_attr, get_rfds, publish_rfd, reserve_rfd, search_rfds, set_rfd_attr, set_rfd_content, set_rfd_document, update_rfd_visibility }, webhook::github_webhook, well_known::{jwks_json, openid_configuration}, @@ -101,6 +100,10 @@ pub fn server( .expect("Failed to register endpoint"); api.register(set_rfd_attr) .expect("Failed to register endpoint"); + api.register(discuss_rfd) + .expect("Failed to register endpoint"); + api.register(publish_rfd) + .expect("Failed to register endpoint"); api.register(update_rfd_visibility) .expect("Failed to register endpoint"); api.register(search_rfds) diff --git a/rfd-cli/src/generated/cli.rs b/rfd-cli/src/generated/cli.rs index 73d84cfa..c4952faa 100644 --- a/rfd-cli/src/generated/cli.rs +++ b/rfd-cli/src/generated/cli.rs @@ -57,6 +57,8 @@ impl Cli { CliCommand::GetRfdAttr => Self::cli_get_rfd_attr(), CliCommand::SetRfdAttr => Self::cli_set_rfd_attr(), CliCommand::SetRfdContent => Self::cli_set_rfd_content(), + CliCommand::DiscussRfd => Self::cli_discuss_rfd(), + CliCommand::PublishRfd => Self::cli_publish_rfd(), CliCommand::UpdateRfdVisibility => Self::cli_update_rfd_visibility(), CliCommand::SearchRfds => Self::cli_search_rfds(), CliCommand::GetSelf => Self::cli_get_self(), @@ -738,7 +740,7 @@ impl Cli { .required(true) .help("The RFD number (examples: 1 or 123)"), ) - .about("Get the latest representation of an RFD") + .about("Get the latest representation of a RFD") } pub fn cli_set_rfd_document() -> clap::Command { @@ -802,7 +804,7 @@ impl Cli { .value_parser(clap::value_parser!(String)) .required(true), ) - .about("Get an attribute of a given RFD") + .about("Get an attribute of a RFD") } pub fn cli_set_rfd_attr() -> clap::Command { @@ -854,7 +856,7 @@ impl Cli { .action(clap::ArgAction::SetTrue) .help("XXX"), ) - .about("Set an attribute of a given RFD") + .about("Set an attribute of a RFD") } pub fn cli_set_rfd_content() -> clap::Command { @@ -894,7 +896,31 @@ impl Cli { .action(clap::ArgAction::SetTrue) .help("XXX"), ) - .about("Replace the contents of an RFD") + .about("Replace the contents of a RFD") + } + + pub fn cli_discuss_rfd() -> clap::Command { + clap::Command::new("") + .arg( + clap::Arg::new("number") + .long("number") + .value_parser(clap::value_parser!(String)) + .required(true) + .help("The RFD number (examples: 1 or 123)"), + ) + .about("Open a RFD for discussion") + } + + pub fn cli_publish_rfd() -> clap::Command { + clap::Command::new("") + .arg( + clap::Arg::new("number") + .long("number") + .value_parser(clap::value_parser!(String)) + .required(true) + .help("The RFD number (examples: 1 or 123)"), + ) + .about("Publish a RFD") } pub fn cli_update_rfd_visibility() -> clap::Command { @@ -932,7 +958,7 @@ impl Cli { .action(clap::ArgAction::SetTrue) .help("XXX"), ) - .about("Modify the visibility of an RFD") + .about("Modify the visibility of a RFD") } pub fn cli_search_rfds() -> clap::Command { @@ -1030,6 +1056,8 @@ impl Cli { CliCommand::GetRfdAttr => self.execute_get_rfd_attr(matches).await, CliCommand::SetRfdAttr => self.execute_set_rfd_attr(matches).await, CliCommand::SetRfdContent => self.execute_set_rfd_content(matches).await, + CliCommand::DiscussRfd => self.execute_discuss_rfd(matches).await, + CliCommand::PublishRfd => self.execute_publish_rfd(matches).await, CliCommand::UpdateRfdVisibility => self.execute_update_rfd_visibility(matches).await, CliCommand::SearchRfds => self.execute_search_rfds(matches).await, CliCommand::GetSelf => self.execute_get_self(matches).await, @@ -2079,6 +2107,46 @@ impl Cli { } } + pub async fn execute_discuss_rfd(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { + let mut request = self.client.discuss_rfd(); + if let Some(value) = matches.get_one::("number") { + request = request.number(value.clone()); + } + + self.config.execute_discuss_rfd(matches, &mut request)?; + let result = request.send().await; + match result { + Ok(r) => { + self.config.item_success(&r); + Ok(()) + } + Err(r) => { + self.config.item_error(&r); + Err(anyhow::Error::new(r)) + } + } + } + + pub async fn execute_publish_rfd(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { + let mut request = self.client.publish_rfd(); + if let Some(value) = matches.get_one::("number") { + request = request.number(value.clone()); + } + + self.config.execute_publish_rfd(matches, &mut request)?; + let result = request.send().await; + match result { + Ok(r) => { + self.config.item_success(&r); + Ok(()) + } + Err(r) => { + self.config.item_error(&r); + Err(anyhow::Error::new(r)) + } + } + } + pub async fn execute_update_rfd_visibility( &self, matches: &clap::ArgMatches, @@ -2493,6 +2561,22 @@ pub trait CliConfig { Ok(()) } + fn execute_discuss_rfd( + &self, + matches: &clap::ArgMatches, + request: &mut builder::DiscussRfd, + ) -> anyhow::Result<()> { + Ok(()) + } + + fn execute_publish_rfd( + &self, + matches: &clap::ArgMatches, + request: &mut builder::PublishRfd, + ) -> anyhow::Result<()> { + Ok(()) + } + fn execute_update_rfd_visibility( &self, matches: &clap::ArgMatches, @@ -2558,6 +2642,8 @@ pub enum CliCommand { GetRfdAttr, SetRfdAttr, SetRfdContent, + DiscussRfd, + PublishRfd, UpdateRfdVisibility, SearchRfds, GetSelf, @@ -2604,6 +2690,8 @@ impl CliCommand { CliCommand::GetRfdAttr, CliCommand::SetRfdAttr, CliCommand::SetRfdContent, + CliCommand::DiscussRfd, + CliCommand::PublishRfd, CliCommand::UpdateRfdVisibility, CliCommand::SearchRfds, CliCommand::GetSelf, diff --git a/rfd-cli/src/main.rs b/rfd-cli/src/main.rs index bb2f3cb7..34cb8701 100644 --- a/rfd-cli/src/main.rs +++ b/rfd-cli/src/main.rs @@ -142,6 +142,8 @@ fn cmd_path<'a>(cmd: &CliCommand) -> Option<&'a str> { CliCommand::SetRfdContent => Some("edit content"), CliCommand::SetRfdDocument => Some("edit document"), CliCommand::UpdateRfdVisibility => Some("edit visibility"), + CliCommand::PublishRfd => Some("edit publish"), + CliCommand::DiscussRfd => Some("edit discuss"), // User commands CliCommand::CreateApiUser => Some("sys user create"), diff --git a/rfd-data/src/lib.rs b/rfd-data/src/lib.rs index 14f926bd..3b419c9c 100644 --- a/rfd-data/src/lib.rs +++ b/rfd-data/src/lib.rs @@ -18,7 +18,7 @@ impl RfdNumber { format!("/rfd/{}", self.as_number_string()) } - /// Get an RFD number in its expanded form with leading 0s + /// Get a RFD number in its expanded form with leading 0s pub fn as_number_string(&self) -> String { let mut number_string = self.0.to_string(); while number_string.len() < 4 { diff --git a/rfd-processor/src/updater/ensure_default_state.rs b/rfd-processor/src/updater/ensure_default_state.rs index 85a0f1f4..cb2d773a 100644 --- a/rfd-processor/src/updater/ensure_default_state.rs +++ b/rfd-processor/src/updater/ensure_default_state.rs @@ -26,7 +26,7 @@ impl RfdUpdateAction for EnsureRfdOnDefaultIsInValidState { ) -> Result { let RfdUpdateActionContext { update, .. } = ctx; - // If an RFD exists on the default branch then it should be in either the published or + // If a RFD exists on the default branch then it should be in either the published or // abandoned state if update.location.branch == update.location.default_branch { if !new.is_state("published") diff --git a/rfd-processor/src/updater/mod.rs b/rfd-processor/src/updater/mod.rs index a437746e..8814eefb 100644 --- a/rfd-processor/src/updater/mod.rs +++ b/rfd-processor/src/updater/mod.rs @@ -64,10 +64,10 @@ trait Validate { impl Validate for GitHubRfdUpdate { fn is_valid(&self) -> bool { - // An RFD update is only valid in one of two cases: + // a RFD update is only valid in one of two cases: // `1. The update is occurring on the default branch. In this case it does not matter what // RFD is being updated, the update is always considered valid - // 2. The update is occurring on an RFD branch with a name of the pattern \d\d\d\d . In + // 2. The update is occurring on a RFD branch with a name of the pattern \d\d\d\d . In // this case, the update is only valid if the number of the RFD being updated matches // the branch the update is occurring on. self.location.branch == self.location.default_branch diff --git a/rfd-processor/src/updater/update_pdfs.rs b/rfd-processor/src/updater/update_pdfs.rs index e4eefea5..684fa8ca 100644 --- a/rfd-processor/src/updater/update_pdfs.rs +++ b/rfd-processor/src/updater/update_pdfs.rs @@ -49,7 +49,7 @@ impl UpdatePdfs { RfdOutputError::FormatNotSupported => { tracing::info!("RFD is not in a format that supports PDF output"); - // If an RFD does not support PDF output than we do not want to report an + // If a RFD does not support PDF output than we do not want to report an // error. We return early instead return Ok(vec![]); } diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index 67f510f3..e7c770fb 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -10187,7 +10187,7 @@ impl Client { builder::ReserveRfd::new(self) } - /// Get the latest representation of an RFD + /// Get the latest representation of a RFD /// /// Sends a `GET` request to `/rfd/{number}` /// @@ -10221,7 +10221,7 @@ impl Client { builder::SetRfdDocument::new(self) } - /// Get an attribute of a given RFD + /// Get an attribute of a RFD /// /// Sends a `GET` request to `/rfd/{number}/attr/{attr}` /// @@ -10236,9 +10236,9 @@ impl Client { builder::GetRfdAttr::new(self) } - /// Set an attribute of a given RFD + /// Set an attribute of a RFD /// - /// Sends a `PUT` request to `/rfd/{number}/attr/{attr}` + /// Sends a `POST` request to `/rfd/{number}/attr/{attr}` /// /// ```ignore /// let response = client.set_rfd_attr() @@ -10252,7 +10252,7 @@ impl Client { builder::SetRfdAttr::new(self) } - /// Replace the contents of an RFD + /// Replace the contents of a RFD /// /// Sends a `POST` request to `/rfd/{number}/content` /// @@ -10270,7 +10270,39 @@ impl Client { builder::SetRfdContent::new(self) } - /// Modify the visibility of an RFD + /// Open a RFD for discussion + /// + /// Sends a `POST` request to `/rfd/{number}/discuss` + /// + /// Arguments: + /// - `number`: The RFD number (examples: 1 or 123) + /// ```ignore + /// let response = client.discuss_rfd() + /// .number(number) + /// .send() + /// .await; + /// ``` + pub fn discuss_rfd(&self) -> builder::DiscussRfd { + builder::DiscussRfd::new(self) + } + + /// Publish a RFD + /// + /// Sends a `POST` request to `/rfd/{number}/publish` + /// + /// Arguments: + /// - `number`: The RFD number (examples: 1 or 123) + /// ```ignore + /// let response = client.publish_rfd() + /// .number(number) + /// .send() + /// .await; + /// ``` + pub fn publish_rfd(&self) -> builder::PublishRfd { + builder::PublishRfd::new(self) + } + + /// Modify the visibility of a RFD /// /// Sends a `POST` request to `/rfd/{number}/visibility` /// @@ -13061,7 +13093,7 @@ pub mod builder { self } - /// Sends a `PUT` request to `/rfd/{number}/attr/{attr}` + /// Sends a `POST` request to `/rfd/{number}/attr/{attr}` pub async fn send(self) -> Result, Error> { let Self { client, @@ -13083,7 +13115,7 @@ pub mod builder { #[allow(unused_mut)] let mut request = client .client - .put(url) + .post(url) .header( reqwest::header::ACCEPT, reqwest::header::HeaderValue::from_static("application/json"), @@ -13199,6 +13231,126 @@ pub mod builder { } } + /// Builder for [`Client::discuss_rfd`] + /// + /// [`Client::discuss_rfd`]: super::Client::discuss_rfd + #[derive(Debug, Clone)] + pub struct DiscussRfd<'a> { + client: &'a super::Client, + number: Result, + } + + impl<'a> DiscussRfd<'a> { + pub fn new(client: &'a super::Client) -> Self { + Self { + client: client, + number: Err("number was not initialized".to_string()), + } + } + + pub fn number(mut self, value: V) -> Self + where + V: std::convert::TryInto, + { + self.number = value + .try_into() + .map_err(|_| "conversion to `String` for number failed".to_string()); + self + } + + /// Sends a `POST` request to `/rfd/{number}/discuss` + pub async fn send(self) -> Result, Error> { + let Self { client, number } = self; + let number = number.map_err(Error::InvalidRequest)?; + let url = format!( + "{}/rfd/{}/discuss", + client.baseurl, + encode_path(&number.to_string()), + ); + #[allow(unused_mut)] + let mut request = client + .client + .post(url) + .header( + reqwest::header::ACCEPT, + reqwest::header::HeaderValue::from_static("application/json"), + ) + .build()?; + let result = client.client.execute(request).await; + let response = result?; + match response.status().as_u16() { + 202u16 => ResponseValue::from_response(response).await, + 400u16..=499u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + 500u16..=599u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + _ => Err(Error::UnexpectedResponse(response)), + } + } + } + + /// Builder for [`Client::publish_rfd`] + /// + /// [`Client::publish_rfd`]: super::Client::publish_rfd + #[derive(Debug, Clone)] + pub struct PublishRfd<'a> { + client: &'a super::Client, + number: Result, + } + + impl<'a> PublishRfd<'a> { + pub fn new(client: &'a super::Client) -> Self { + Self { + client: client, + number: Err("number was not initialized".to_string()), + } + } + + pub fn number(mut self, value: V) -> Self + where + V: std::convert::TryInto, + { + self.number = value + .try_into() + .map_err(|_| "conversion to `String` for number failed".to_string()); + self + } + + /// Sends a `POST` request to `/rfd/{number}/publish` + pub async fn send(self) -> Result, Error> { + let Self { client, number } = self; + let number = number.map_err(Error::InvalidRequest)?; + let url = format!( + "{}/rfd/{}/publish", + client.baseurl, + encode_path(&number.to_string()), + ); + #[allow(unused_mut)] + let mut request = client + .client + .post(url) + .header( + reqwest::header::ACCEPT, + reqwest::header::HeaderValue::from_static("application/json"), + ) + .build()?; + let result = client.client.execute(request).await; + let response = result?; + match response.status().as_u16() { + 202u16 => ResponseValue::from_response(response).await, + 400u16..=499u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + 500u16..=599u16 => Err(Error::ErrorResponse( + ResponseValue::from_response(response).await?, + )), + _ => Err(Error::UnexpectedResponse(response)), + } + } + } + /// Builder for [`Client::update_rfd_visibility`] /// /// [`Client::update_rfd_visibility`]: super::Client::update_rfd_visibility From b8da3b42cae3be21ebebc1289b10c642bc5183d4 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 22 Apr 2024 10:34:53 -0500 Subject: [PATCH 07/26] Help text fixes --- rfd-api-spec.json | 2 +- rfd-api/src/endpoints/rfd.rs | 2 +- rfd-cli/src/generated/cli.rs | 2 +- rfd-sdk/src/generated/sdk.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/rfd-api-spec.json b/rfd-api-spec.json index 84ff9b22..5960abbf 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -1285,7 +1285,7 @@ } }, "post": { - "summary": "Replace the full document of for a RFD", + "summary": "Replace the full document of a RFD", "operationId": "set_rfd_document", "parameters": [ { diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 5bfd4a5d..2cdf7d7e 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -150,7 +150,7 @@ pub struct RfdUpdateBody { message: Option, } -/// Replace the full document of for a RFD +/// Replace the full document of a RFD #[trace_request] #[endpoint { method = POST, diff --git a/rfd-cli/src/generated/cli.rs b/rfd-cli/src/generated/cli.rs index c4952faa..88d6f2f5 100644 --- a/rfd-cli/src/generated/cli.rs +++ b/rfd-cli/src/generated/cli.rs @@ -780,7 +780,7 @@ impl Cli { .action(clap::ArgAction::SetTrue) .help("XXX"), ) - .about("Replace the full document of for a RFD") + .about("Replace the full document of a RFD") } pub fn cli_get_rfd_attr() -> clap::Command { diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index e7c770fb..254dd449 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -10203,7 +10203,7 @@ impl Client { builder::GetRfd::new(self) } - /// Replace the full document of for a RFD + /// Replace the full document of a RFD /// /// Sends a `POST` request to `/rfd/{number}` /// From 7350f1d8b45b76f1b7b79bf2ef0fe04391cd871a Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 22 Apr 2024 10:35:14 -0500 Subject: [PATCH 08/26] Fmt --- rfd-api/src/endpoints/rfd.rs | 10 ++++++++-- rfd-api/src/server.rs | 3 ++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 2cdf7d7e..57e9dcb6 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -422,7 +422,10 @@ pub async fn discuss_rfd( &ctx.get_caller(auth.as_ref()).await?, path.number, RfdAttrName::State, - &RfdAttrValue { value: RfdState::Discussion.to_string(), message: Some("Move to discussion".to_string()) }, + &RfdAttrValue { + value: RfdState::Discussion.to_string(), + message: Some("Move to discussion".to_string()), + }, ) .await } @@ -446,7 +449,10 @@ pub async fn publish_rfd( &ctx.get_caller(auth.as_ref()).await?, path.number, RfdAttrName::State, - &RfdAttrValue { value: RfdState::Published.to_string(), message: Some("Publish".to_string()) }, + &RfdAttrValue { + value: RfdState::Published.to_string(), + message: Some("Publish".to_string()), + }, ) .await } diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index bb34ea57..8027c017 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -30,7 +30,8 @@ use crate::{ }, mappers::{create_mapper, delete_mapper, get_mappers}, rfd::{ - discuss_rfd, get_rfd, get_rfd_attr, get_rfds, publish_rfd, reserve_rfd, search_rfds, set_rfd_attr, set_rfd_content, set_rfd_document, update_rfd_visibility + discuss_rfd, get_rfd, get_rfd_attr, get_rfds, publish_rfd, reserve_rfd, search_rfds, + set_rfd_attr, set_rfd_content, set_rfd_document, update_rfd_visibility, }, webhook::github_webhook, well_known::{jwks_json, openid_configuration}, From 4bf5e0dafdf394c45df373ef6ec24dea914b996d Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 22 Apr 2024 10:35:40 -0500 Subject: [PATCH 09/26] License fix --- rfd-data/src/content/template.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rfd-data/src/content/template.rs b/rfd-data/src/content/template.rs index 74f70c14..dd8512cd 100644 --- a/rfd-data/src/content/template.rs +++ b/rfd-data/src/content/template.rs @@ -1,3 +1,7 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + use serde::{Deserialize, Serialize}; use std::collections::HashMap; use thiserror::Error; From 504de888872b084978de4971d253caa68f2e9669 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 22 Apr 2024 10:46:11 -0500 Subject: [PATCH 10/26] Fix test context --- rfd-api/src/context.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index b232608c..8a6214e2 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -2275,6 +2275,9 @@ pub(crate) mod test_mocks { content .templates .insert("new".to_string(), RfdTemplate::default()); + content + .templates + .insert("placeholder".to_string(), RfdTemplate::default()); let mut ctx = ApiContext::new( "".to_string(), From 45b7c443f2c2920f1142e39fd53606c28270a824 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 26 Apr 2024 11:51:05 -0500 Subject: [PATCH 11/26] Update example config files --- rfd-api/config.example.toml | 64 +++++++++++++++++++++++-- rfd-processor/config.example.toml | 79 +++++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+), 5 deletions(-) create mode 100644 rfd-processor/config.example.toml diff --git a/rfd-api/config.example.toml b/rfd-api/config.example.toml index a0066478..6171a84c 100644 --- a/rfd-api/config.example.toml +++ b/rfd-api/config.example.toml @@ -17,7 +17,7 @@ public_url = "" server_port = 8080 # Full url of the Postgres database to connect to -database_url = "" +database_url = "postgres://:@/" # Settings for JWT management [jwt] @@ -26,7 +26,7 @@ database_url = "" default_expiration = 3600 # Keys for signing JWTs and generating secrets. Currently GCP Cloud KMS keys and local static keys -# are supported +# are supported. At least one key must be configured. # Cloud KMS [[keys]] @@ -46,7 +46,8 @@ private = """""" # PEM encoded private key public = """""" # PEM encoded public key # OAuth Providers -# Google and GitHub are supported. An OAuth provider needs to have both a web and device config +# Google and GitHub are supported. An OAuth provider needs to have both a web and device config. +# At least one OAuth provider must be configured [authn.oauth.google.device] client_id = "" @@ -55,7 +56,16 @@ client_secret = "" [authn.oauth.google.web] client_id = "" client_secret = "" -redirect_uri = "" +redirect_uri = "https:///login/oauth/google/code/callback" + +[authn.oauth.github.device] +client_id = "" +client_secret = "" + +[authn.oauth.github.web] +client_id = "" +client_secret = "" +redirect_uri = "https:///login/oauth/github/code/callback" # Search configuration [search] @@ -64,7 +74,7 @@ host = "" # Read-only search key key = "" # Index to perform searches against -index = "rfd" +index = "" # Fields for use in generating the OpenAPI spec file [spec] @@ -73,3 +83,47 @@ description = "" contact_url = "" contact_email = "" output_path = "" + +# Templated for creating new RFDs. The 'placeholder' and 'new' templates are the only two templates +# available and are both required + +# Template used when creating a new RFD without specifying a body +[content.templates.placeholder] +template = """""" +required_fields = [] + +# Template used when creating a new RFD while specifying a body +[content.templates.new] +template = """""" +required_fields = [] + +# The GitHub repository to use to write RFDs +[source] +# GitHub user or organization +owner = "" +# GitHub repository name +repo = "" +# Path within the repository where RFDs are stored +path = "" +# Branch to use as the default branch of ther repository +default_branch = "" + +# The method for authenticating to GitHub. This requires one of two authentication styles: +# 1. A GitHub App installation that is defined by an app_id, installation_id, and private_key +# 2. A GitHub access token +# Exactly one authentication must be specified + +# App Installation +[services.github.auth] +# Numeric GitHub App id +app_id = 1111111 +# Numeric GitHub App installation id corresponding to the organization that the configured repo +# belongs to +installation_id = 2222222 +# PEM encoded private key for the GitHub App +private_key = """""" + +# Access Token +[services.github.auth] +# This may be any GitHub access token that has permission to the configured repo +token = "" diff --git a/rfd-processor/config.example.toml b/rfd-processor/config.example.toml new file mode 100644 index 00000000..c6961e40 --- /dev/null +++ b/rfd-processor/config.example.toml @@ -0,0 +1,79 @@ +# How many jobs should be processed at once +processor_batch_size = 10 + +# How often to select a batch of jobs to process +processor_interval = 30 + +# A control mode for all processor actions, designating if the action should persist data to remote +# services or only generate what would be persisted. +processor_update_mode = "read" + +# How often the processor scanner should check the remote GitHub repo for RFDs +scanner_interval = 900 + +# The internal database url to store RFD information +database_url = "postgres://:@/" + +# The list of actions that should be run for each processing job +actions = [ + # "CopyImagesToStorage", + # "UpdateSearch", + # "UpdatePdfs", + # "CreatePullRequest", + # "UpdatePullRequest", + # "UpdateDiscussionUrl", + # "EnsureRfdWithPullRequestIsInValidState", + # "EnsureRfdOnDefaultIsInValidState", +] + +# The method for authenticating to GitHub. This requires one of two authentication styles: +# 1. A GitHub App installation that is defined by an app_id, installation_id, and private_key +# 2. A GitHub access token +# Exactly one authentication must be specified + +# App Installation +[auth.github] +# Numeric GitHub App id +app_id = 1111111 +# Numeric GitHub App installation id corresponding to the organization that the configured repo +# belongs to +installation_id = 2222222 +# PEM encoded private key for the GitHub App +private_key = """""" + +# Access Token +[auth.github] +# This may be any GitHub access token that has permission to the configured repo +token = "" + +# The GitHub repository to use to read and write RFDs +[source] +# GitHub user or organization +owner = "" +# GitHub repository name +repo = "" +# Path within the repository where RFDs are stored +path = "" +# Branch to use as the default branch of ther repository +default_branch = "" + +# Bucket to push static assets pulled from RFDs to (currently only GCP Storage buckets are supported) +[[static_storage]] +# Name of the bucket +bucket = "" + +# Location to store generated PDFs (currently on Google Drive Shared Drives are supported) +[pdf_storage] +# Shared Drive id +drive = "" +# Folder id within the Shared Drive +folder = "" + +# Search backend for indexing RFD contents (currently on Meilisearch is supported) +[[search_storage]] +# Https endpoint of the search instance +host = "" +# API Key for reading and writing documents +key = "" +# Search index to store documents in +index = "" \ No newline at end of file From a3d1209fe340dedfd76401c7cd288f2d86492162 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 26 Apr 2024 16:07:30 -0500 Subject: [PATCH 12/26] Add local dev login for testing --- .github/workflows/build.yml | 4 +- Cargo.lock | 1 + rfd-api-spec.json | 40 +++++ rfd-api/Cargo.toml | 3 +- rfd-api/src/endpoints/login/local/mod.rs | 89 ++++++++++ rfd-api/src/endpoints/login/mod.rs | 14 ++ .../src/endpoints/login/oauth/device_token.rs | 13 +- rfd-api/src/server.rs | 21 ++- rfd-cli/Cargo.toml | 4 +- rfd-cli/src/cmd/auth/login.rs | 114 ++++++++++-- rfd-cli/src/generated/cli.rs | 70 ++++++++ rfd-cli/src/main.rs | 1 + rfd-sdk/src/generated/sdk.rs | 167 ++++++++++++++++++ 13 files changed, 502 insertions(+), 39 deletions(-) create mode 100644 rfd-api/src/endpoints/login/local/mod.rs diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 5b7455e4..6435daa4 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -15,6 +15,4 @@ jobs: - uses: actions-rs/cargo@844f36862e911db73fe0815f00a4a2602c279505 # v1 with: command: build - args: --release - env: - RUSTFLAGS: "--cfg tracing_unstable" \ No newline at end of file + args: --release --all-features diff --git a/Cargo.lock b/Cargo.lock index d4d08b58..c5e97cfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2567,6 +2567,7 @@ dependencies = [ "clap", "config", "dirs 5.0.1", + "futures", "itertools 0.12.1", "jsonwebtoken 9.2.0", "oauth2", diff --git a/rfd-api-spec.json b/rfd-api-spec.json index 5960abbf..a3a6b68f 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -594,6 +594,31 @@ } } }, + "/login/local": { + "post": { + "operationId": "local_login", + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/LocalLogin" + } + } + }, + "required": true + }, + "responses": { + "default": { + "description": "", + "content": { + "*/*": { + "schema": {} + } + } + } + } + } + }, "/login/oauth/{provider}/code/authorize": { "get": { "summary": "Generate the remote provider login url and redirect the user", @@ -4246,6 +4271,21 @@ "visibility" ] }, + "LocalLogin": { + "type": "object", + "properties": { + "email": { + "type": "string" + }, + "external_id": { + "type": "string" + } + }, + "required": [ + "email", + "external_id" + ] + }, "Mapper": { "type": "object", "properties": { diff --git a/rfd-api/Cargo.toml b/rfd-api/Cargo.toml index a91d66cc..fd2aa182 100644 --- a/rfd-api/Cargo.toml +++ b/rfd-api/Cargo.toml @@ -4,7 +4,8 @@ version = "0.7.0" edition = "2021" repository = "https://github.com/oxidecomputer/rfd-api" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[features] +local-dev = [] [dependencies] anyhow = { workspace = true } diff --git a/rfd-api/src/endpoints/login/local/mod.rs b/rfd-api/src/endpoints/login/local/mod.rs new file mode 100644 index 00000000..94850929 --- /dev/null +++ b/rfd-api/src/endpoints/login/local/mod.rs @@ -0,0 +1,89 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use dropshot::{endpoint, HttpError, RequestContext, TypedBody}; +use http::Response; +use hyper::Body; +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; +use tracing::instrument; + +use crate::context::ApiContext; + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct LocalLogin { + pub external_id: String, + pub email: String, +} + +#[endpoint { + method = POST, + path = "/login/local" +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn local_login( + rqctx: RequestContext, + body: TypedBody, +) -> Result, HttpError> { + #[cfg(feature = "local-dev")] + { + use chrono::Utc; + use http::{header, StatusCode}; + + use crate::endpoints::login::{DeviceTokenResponse, UserInfo}; + + let ctx = rqctx.context(); + let body = body.into_inner(); + + let info = UserInfo { + external_id: super::ExternalUserId::Local(body.external_id), + verified_emails: vec![body.email], + github_username: None, + }; + + let (api_user, api_user_provider) = ctx + .register_api_user(ctx.builtin_registration_user(), info) + .await?; + + tracing::info!(api_user_id = ?api_user.id, api_user_provider_id = ?api_user_provider.id, "Retrieved api user to generate device token for"); + + let token = ctx + .register_access_token( + ctx.builtin_registration_user(), + &api_user, + &api_user_provider, + None, + ) + .await?; + + tracing::info!(provider = "local", api_user_id = ?api_user.id, "Generated access token"); + + Ok(Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "application/json") + .body( + serde_json::to_string(&DeviceTokenResponse { + access_token: token.signed_token, + token_type: "Bearer".to_string(), + expires_in: Some( + (token.expires_at - Utc::now()) + .num_seconds() + .try_into() + .unwrap_or(0), + ), + refresh_token: None, + scopes: None, + }) + .unwrap() + .into(), + )?) + } + #[cfg(not(feature = "local-dev"))] + { + Err(HttpError::for_not_found( + None, + "Local login is not supported".to_string(), + )) + } +} diff --git a/rfd-api/src/endpoints/login/mod.rs b/rfd-api/src/endpoints/login/mod.rs index bc16522e..c140e5de 100644 --- a/rfd-api/src/endpoints/login/mod.rs +++ b/rfd-api/src/endpoints/login/mod.rs @@ -16,6 +16,7 @@ use crate::{ util::response::{bad_request, internal_error}, }; +pub mod local; pub mod oauth; #[derive(Debug, Deserialize, Serialize, JsonSchema)] @@ -57,6 +58,7 @@ impl From for HttpError { pub enum ExternalUserId { GitHub(String), Google(String), + Local(String), } impl ExternalUserId { @@ -64,6 +66,7 @@ impl ExternalUserId { match self { Self::GitHub(id) => id, Self::Google(id) => id, + Self::Local(id) => id, } } @@ -71,6 +74,7 @@ impl ExternalUserId { match self { Self::GitHub(_) => "github", Self::Google(_) => "google", + Self::Local(_) => "local", } } } @@ -91,6 +95,7 @@ impl Serialize for ExternalUserId { match self { ExternalUserId::GitHub(id) => serializer.serialize_str(&format!("github-{}", id)), ExternalUserId::Google(id) => serializer.serialize_str(&format!("google-{}", id)), + ExternalUserId::Local(id) => serializer.serialize_str(&format!("local-{}", id)), } } } @@ -166,3 +171,12 @@ pub trait UserInfoProvider { token: &str, ) -> Result; } + +#[derive(Debug, Deserialize, JsonSchema, Serialize)] +pub struct DeviceTokenResponse { + pub access_token: String, + pub token_type: String, + pub expires_in: Option, + pub refresh_token: Option, + pub scopes: Option>, +} diff --git a/rfd-api/src/endpoints/login/oauth/device_token.rs b/rfd-api/src/endpoints/login/oauth/device_token.rs index 5c4bee48..909b51e2 100644 --- a/rfd-api/src/endpoints/login/oauth/device_token.rs +++ b/rfd-api/src/endpoints/login/oauth/device_token.rs @@ -19,7 +19,7 @@ use super::{ ClientType, OAuthProvider, OAuthProviderInfo, OAuthProviderNameParam, UserInfoProvider, }; use crate::{ - context::ApiContext, endpoints::login::LoginError, error::ApiError, util::response::bad_request, + context::ApiContext, endpoints::login::{DeviceTokenResponse, LoginError}, error::ApiError, util::response::bad_request, }; // Get the metadata about an OAuth provider necessary to begin a device code exchange @@ -89,15 +89,6 @@ impl AccessTokenExchange { } } -#[derive(Debug, Deserialize, JsonSchema, Serialize)] -pub struct ProxyTokenResponse { - access_token: String, - token_type: String, - expires_in: Option, - refresh_token: Option, - scopes: Option>, -} - #[derive(Debug, Deserialize, JsonSchema, Serialize)] pub struct ProxyTokenError { error: String, @@ -211,7 +202,7 @@ pub async fn exchange_device_token( .status(StatusCode::OK) .header(header::CONTENT_TYPE, "application/json") .body( - serde_json::to_string(&ProxyTokenResponse { + serde_json::to_string(&DeviceTokenResponse { access_token: token.signed_token, token_type: "Bearer".to_string(), expires_in: Some( diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index 8027c017..dd47866e 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -19,14 +19,17 @@ use crate::{ remove_api_user_from_group, update_api_user, }, group::{create_group, delete_group, get_groups, update_group}, - login::oauth::{ - client::{ - create_oauth_client, create_oauth_client_redirect_uri, create_oauth_client_secret, - delete_oauth_client_redirect_uri, delete_oauth_client_secret, get_oauth_client, - list_oauth_clients, + login::{ + local::local_login, + oauth::{ + client::{ + create_oauth_client, create_oauth_client_redirect_uri, + create_oauth_client_secret, delete_oauth_client_redirect_uri, + delete_oauth_client_secret, get_oauth_client, list_oauth_clients, + }, + code::{authz_code_callback, authz_code_exchange, authz_code_redirect}, + device_token::{exchange_device_token, get_device_provider}, }, - code::{authz_code_callback, authz_code_exchange, authz_code_redirect}, - device_token::{exchange_device_token, get_device_provider}, }, mappers::{create_mapper, delete_mapper, get_mappers}, rfd::{ @@ -183,6 +186,10 @@ pub fn server( api.register(exchange_device_token) .expect("Failed to register endpoint"); + // Development + api.register(local_login) + .expect("Failed to register endpoint"); + if let Some(spec) = config.spec_output { // Create the API schema. let mut api_definition = &mut api.openapi(spec.title, &""); diff --git a/rfd-cli/Cargo.toml b/rfd-cli/Cargo.toml index b9734461..483e4877 100644 --- a/rfd-cli/Cargo.toml +++ b/rfd-cli/Cargo.toml @@ -3,7 +3,8 @@ name = "rfd-cli" version = "0.7.0" edition = "2021" -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +[features] +local-dev = [] [dependencies] anyhow = { workspace = true } @@ -11,6 +12,7 @@ chrono = { workspace = true } clap = { workspace = true } config = { workspace = true } dirs = { workspace = true } +futures = { workspace = true } itertools = { workspace = true } jsonwebtoken = { workspace = true } oauth2 = { workspace = true } diff --git a/rfd-cli/src/cmd/auth/login.rs b/rfd-cli/src/cmd/auth/login.rs index e24568e4..08702a98 100644 --- a/rfd-cli/src/cmd/auth/login.rs +++ b/rfd-cli/src/cmd/auth/login.rs @@ -7,7 +7,8 @@ use std::ops::Add; use anyhow::Result; use chrono::{Duration, NaiveDate, Utc}; use clap::{Parser, Subcommand, ValueEnum}; -use oauth2::TokenResponse; +use futures::stream::StreamExt; +use oauth2::{basic::BasicTokenType, EmptyExtraTokenFields, StandardTokenResponse, TokenResponse}; use rfd_sdk::types::{ApiPermission, OAuthProviderName}; use crate::{cmd::auth::oauth, Context}; @@ -16,8 +17,8 @@ use crate::{cmd::auth::oauth, Context}; #[derive(Parser, Debug, Clone)] #[clap(name = "login")] pub struct Login { - #[arg(value_enum)] - provider: LoginProvider, + #[command(subcommand)] + provider: LoginProviderCommand, #[arg(short = 'm', default_value = "id")] mode: AuthenticationMode, } @@ -33,14 +34,21 @@ impl Login { } } -#[derive(ValueEnum, Debug, Clone)] -pub enum LoginProvider { +#[derive(Debug, Clone, Subcommand)] +pub enum LoginProviderCommand { + #[clap(name = "github")] /// Login via GitHub - #[value(name = "github")] GitHub, - #[value(name = "google")] /// Login via Google Google, + /// Login with arbitrary details for local development + #[cfg(feature = "local-dev")] + Local { + /// The email to authenticate as + email: String, + /// An arbitrary external id to uniquely identify this user + external_id: String, + }, } #[derive(ValueEnum, Debug, Clone, PartialEq)] @@ -55,19 +63,22 @@ pub enum AuthenticationMode { Token, } -impl LoginProvider { - fn as_name(&self) -> OAuthProviderName { - match self { - Self::GitHub => OAuthProviderName::Github, - Self::Google => OAuthProviderName::Google, - } - } +pub struct OAuthProviderRunner(OAuthProviderName); +pub struct LocalProviderRunner { + email: String, + external_id: String, +} + +pub trait ProviderRunner { + async fn run(&self, ctx: &mut Context, mode: &AuthenticationMode) -> Result; +} - pub async fn run(&self, ctx: &mut Context, mode: &AuthenticationMode) -> Result { +impl ProviderRunner for OAuthProviderRunner { + async fn run(&self, ctx: &mut Context, mode: &AuthenticationMode) -> Result { let provider = ctx .client()? .get_device_provider() - .provider(self.as_name()) + .provider(self.0.to_string()) .send() .await?; @@ -103,3 +114,74 @@ impl LoginProvider { } } } + +impl ProviderRunner for LocalProviderRunner { + async fn run(&self, ctx: &mut Context, mode: &AuthenticationMode) -> Result { + let identity_token = ctx + .client()? + .local_login() + .body_map(|body| { + body.email(self.email.clone()) + .external_id(self.external_id.clone()) + }) + .send() + .await? + .into_inner(); + + let mut bytes = identity_token.into_inner(); + + let mut data = vec![]; + while let Some(chunk) = bytes.next().await { + data.append(&mut chunk?.to_vec()); + } + + let identity_token = match serde_json::from_slice::< + StandardTokenResponse, + >(&data) + { + Ok(token) => Ok(token.access_token().to_owned()), + Err(err) => Err(anyhow::anyhow!("Authentication failed: {}", err)), + }?; + + if mode == &AuthenticationMode::Token { + let client = ctx.new_client(Some(identity_token.secret()))?; + let user = client.get_self().send().await?; + Ok(client + .create_api_user_token() + .identifier(user.info.id) + .body_map(|body| body.expires_at(Utc::now().add(Duration::days(365)))) + .send() + .await? + .key + .to_string()) + } else { + Ok(identity_token.secret().to_string()) + } + } +} + +impl ProviderRunner for LoginProviderCommand { + async fn run(&self, ctx: &mut Context, mode: &AuthenticationMode) -> Result { + match self { + LoginProviderCommand::GitHub => { + OAuthProviderRunner(OAuthProviderName::Github) + .run(ctx, mode) + .await + } + LoginProviderCommand::Google => { + OAuthProviderRunner(OAuthProviderName::Google) + .run(ctx, mode) + .await + } + #[cfg(feature = "local-dev")] + LoginProviderCommand::Local { email, external_id } => { + LocalProviderRunner { + email: email.to_string(), + external_id: external_id.to_string(), + } + .run(ctx, mode) + .await + } + } + } +} diff --git a/rfd-cli/src/generated/cli.rs b/rfd-cli/src/generated/cli.rs index 88d6f2f5..f6a20ffb 100644 --- a/rfd-cli/src/generated/cli.rs +++ b/rfd-cli/src/generated/cli.rs @@ -31,6 +31,7 @@ impl Cli { CliCommand::CreateGroup => Self::cli_create_group(), CliCommand::UpdateGroup => Self::cli_update_group(), CliCommand::DeleteGroup => Self::cli_delete_group(), + CliCommand::LocalLogin => Self::cli_local_login(), CliCommand::AuthzCodeRedirect => Self::cli_authz_code_redirect(), CliCommand::AuthzCodeCallback => Self::cli_authz_code_callback(), CliCommand::AuthzCodeExchange => Self::cli_authz_code_exchange(), @@ -338,6 +339,36 @@ impl Cli { ) } + pub fn cli_local_login() -> clap::Command { + clap::Command::new("") + .arg( + clap::Arg::new("email") + .long("email") + .value_parser(clap::value_parser!(String)) + .required_unless_present("json-body"), + ) + .arg( + clap::Arg::new("external-id") + .long("external-id") + .value_parser(clap::value_parser!(String)) + .required_unless_present("json-body"), + ) + .arg( + clap::Arg::new("json-body") + .long("json-body") + .value_name("JSON-FILE") + .required(false) + .value_parser(clap::value_parser!(std::path::PathBuf)) + .help("Path to a file that contains the full json body."), + ) + .arg( + clap::Arg::new("json-body-template") + .long("json-body-template") + .action(clap::ArgAction::SetTrue) + .help("XXX"), + ) + } + pub fn cli_authz_code_redirect() -> clap::Command { clap::Command::new("") .arg( @@ -1026,6 +1057,7 @@ impl Cli { CliCommand::CreateGroup => self.execute_create_group(matches).await, CliCommand::UpdateGroup => self.execute_update_group(matches).await, CliCommand::DeleteGroup => self.execute_delete_group(matches).await, + CliCommand::LocalLogin => self.execute_local_login(matches).await, CliCommand::AuthzCodeRedirect => self.execute_authz_code_redirect(matches).await, CliCommand::AuthzCodeCallback => self.execute_authz_code_callback(matches).await, CliCommand::AuthzCodeExchange => self.execute_authz_code_exchange(matches).await, @@ -1465,6 +1497,34 @@ impl Cli { } } + pub async fn execute_local_login(&self, matches: &clap::ArgMatches) -> anyhow::Result<()> { + let mut request = self.client.local_login(); + if let Some(value) = matches.get_one::("email") { + request = request.body_map(|body| body.email(value.clone())) + } + + if let Some(value) = matches.get_one::("external-id") { + request = request.body_map(|body| body.external_id(value.clone())) + } + + if let Some(value) = matches.get_one::("json-body") { + let body_txt = std::fs::read_to_string(value).unwrap(); + let body_value = serde_json::from_str::(&body_txt).unwrap(); + request = request.body(body_value); + } + + self.config.execute_local_login(matches, &mut request)?; + let result = request.send().await; + match result { + Ok(r) => { + todo!() + } + Err(r) => { + todo!() + } + } + } + pub async fn execute_authz_code_redirect( &self, matches: &clap::ArgMatches, @@ -2385,6 +2445,14 @@ pub trait CliConfig { Ok(()) } + fn execute_local_login( + &self, + matches: &clap::ArgMatches, + request: &mut builder::LocalLogin, + ) -> anyhow::Result<()> { + Ok(()) + } + fn execute_authz_code_redirect( &self, matches: &clap::ArgMatches, @@ -2620,6 +2688,7 @@ pub enum CliCommand { CreateGroup, UpdateGroup, DeleteGroup, + LocalLogin, AuthzCodeRedirect, AuthzCodeCallback, AuthzCodeExchange, @@ -2668,6 +2737,7 @@ impl CliCommand { CliCommand::CreateGroup, CliCommand::UpdateGroup, CliCommand::DeleteGroup, + CliCommand::LocalLogin, CliCommand::AuthzCodeRedirect, CliCommand::AuthzCodeCallback, CliCommand::AuthzCodeExchange, diff --git a/rfd-cli/src/main.rs b/rfd-cli/src/main.rs index 34cb8701..94c6d581 100644 --- a/rfd-cli/src/main.rs +++ b/rfd-cli/src/main.rs @@ -188,6 +188,7 @@ fn cmd_path<'a>(cmd: &CliCommand) -> Option<&'a str> { CliCommand::GetDeviceProvider => None, // Unsupported commands + CliCommand::LocalLogin => None, CliCommand::AuthzCodeRedirect => None, CliCommand::AuthzCodeCallback => None, CliCommand::AuthzCodeExchange => None, diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index 254dd449..ad963594 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -3950,6 +3950,49 @@ pub mod types { } } + /// LocalLogin + /// + ///
JSON schema + /// + /// ```json + /// { + /// "type": "object", + /// "required": [ + /// "email", + /// "external_id" + /// ], + /// "properties": { + /// "email": { + /// "type": "string" + /// }, + /// "external_id": { + /// "type": "string" + /// } + + /// } + + /// } + + /// ``` + ///
+ #[derive(Clone, Debug, Deserialize, Serialize, schemars :: JsonSchema)] + pub struct LocalLogin { + pub email: String, + pub external_id: String, + } + + impl From<&LocalLogin> for LocalLogin { + fn from(value: &LocalLogin) -> Self { + value.clone() + } + } + + impl LocalLogin { + pub fn builder() -> builder::LocalLogin { + Default::default() + } + } + /// Mapper /// ///
JSON schema @@ -8198,6 +8241,63 @@ pub mod types { } } + #[derive(Clone, Debug)] + pub struct LocalLogin { + email: Result, + external_id: Result, + } + + impl Default for LocalLogin { + fn default() -> Self { + Self { + email: Err("no value supplied for email".to_string()), + external_id: Err("no value supplied for external_id".to_string()), + } + } + } + + impl LocalLogin { + pub fn email(mut self, value: T) -> Self + where + T: std::convert::TryInto, + T::Error: std::fmt::Display, + { + self.email = value + .try_into() + .map_err(|e| format!("error converting supplied value for email: {}", e)); + self + } + pub fn external_id(mut self, value: T) -> Self + where + T: std::convert::TryInto, + T::Error: std::fmt::Display, + { + self.external_id = value + .try_into() + .map_err(|e| format!("error converting supplied value for external_id: {}", e)); + self + } + } + + impl std::convert::TryFrom for super::LocalLogin { + type Error = super::error::ConversionError; + fn try_from(value: LocalLogin) -> Result { + Ok(Self { + email: value.email?, + external_id: value.external_id?, + }) + } + } + + impl From for LocalLogin { + fn from(value: super::LocalLogin) -> Self { + Self { + email: Ok(value.email), + external_id: Ok(value.external_id), + } + } + } + #[derive(Clone, Debug)] pub struct Mapper { activations: Result, String>, @@ -9945,6 +10045,18 @@ impl Client { builder::DeleteGroup::new(self) } + /// Sends a `POST` request to `/login/local` + /// + /// ```ignore + /// let response = client.local_login() + /// .body(body) + /// .send() + /// .await; + /// ``` + pub fn local_login(&self) -> builder::LocalLogin { + builder::LocalLogin::new(self) + } + /// Generate the remote provider login url and redirect the user /// /// Sends a `GET` request to `/login/oauth/{provider}/code/authorize` @@ -11545,6 +11657,61 @@ pub mod builder { } } + /// Builder for [`Client::local_login`] + /// + /// [`Client::local_login`]: super::Client::local_login + #[derive(Debug, Clone)] + pub struct LocalLogin<'a> { + client: &'a super::Client, + body: Result, + } + + impl<'a> LocalLogin<'a> { + pub fn new(client: &'a super::Client) -> Self { + Self { + client: client, + body: Ok(types::builder::LocalLogin::default()), + } + } + + pub fn body(mut self, value: V) -> Self + where + V: std::convert::TryInto, + >::Error: std::fmt::Display, + { + self.body = value + .try_into() + .map(From::from) + .map_err(|s| format!("conversion to `LocalLogin` for body failed: {}", s)); + self + } + + pub fn body_map(mut self, f: F) -> Self + where + F: std::ops::FnOnce(types::builder::LocalLogin) -> types::builder::LocalLogin, + { + self.body = self.body.map(f); + self + } + + /// Sends a `POST` request to `/login/local` + pub async fn send(self) -> Result, Error> { + let Self { client, body } = self; + let body = body + .and_then(|v| types::LocalLogin::try_from(v).map_err(|e| e.to_string())) + .map_err(Error::InvalidRequest)?; + let url = format!("{}/login/local", client.baseurl,); + #[allow(unused_mut)] + let mut request = client.client.post(url).json(&body).build()?; + let result = client.client.execute(request).await; + let response = result?; + match response.status().as_u16() { + 200..=299 => Ok(ResponseValue::stream(response)), + _ => Err(Error::ErrorResponse(ResponseValue::stream(response))), + } + } + } + /// Builder for [`Client::authz_code_redirect`] /// /// [`Client::authz_code_redirect`]: super::Client::authz_code_redirect From d8aebb4481990d646419614e322593199aa943f1 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Fri, 26 Apr 2024 16:19:10 -0500 Subject: [PATCH 13/26] Fmt --- rfd-api/src/endpoints/login/oauth/device_token.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/rfd-api/src/endpoints/login/oauth/device_token.rs b/rfd-api/src/endpoints/login/oauth/device_token.rs index 909b51e2..c1a1d591 100644 --- a/rfd-api/src/endpoints/login/oauth/device_token.rs +++ b/rfd-api/src/endpoints/login/oauth/device_token.rs @@ -19,7 +19,10 @@ use super::{ ClientType, OAuthProvider, OAuthProviderInfo, OAuthProviderNameParam, UserInfoProvider, }; use crate::{ - context::ApiContext, endpoints::login::{DeviceTokenResponse, LoginError}, error::ApiError, util::response::bad_request, + context::ApiContext, + endpoints::login::{DeviceTokenResponse, LoginError}, + error::ApiError, + util::response::bad_request, }; // Get the metadata about an OAuth provider necessary to begin a device code exchange From 1d981eba080c484d9ea01c94547dc1e09f5639dc Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 29 Apr 2024 11:35:46 -0500 Subject: [PATCH 14/26] Add job immediately after performing updates --- rfd-api/src/context.rs | 51 +++++++++++++++++++++++++++++++++--------- rfd-github/src/lib.rs | 9 ++++---- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 8a6214e2..6fad135c 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -931,7 +931,7 @@ impl ApiContext { content: &str, message: Option<&str>, branch_name: Option<&str>, - ) -> ResourceResult<(), UpdateRfdContentError> { + ) -> ResourceResult, UpdateRfdContentError> { if caller.any(&[ &ApiPermission::UpdateRfd(rfd_number), &ApiPermission::UpdateRfdsAll, @@ -947,7 +947,7 @@ impl ApiContext { self.commit_rfd_document( caller, - rfd_number, + rfd_number.into(), updated_content.raw(), message, &sha, @@ -967,7 +967,7 @@ impl ApiContext { document: &str, message: Option<&str>, branch_name: Option<&str>, - ) -> ResourceResult<(), UpdateRfdContentError> { + ) -> ResourceResult, UpdateRfdContentError> { if caller.any(&[ &ApiPermission::UpdateRfd(rfd_number), &ApiPermission::UpdateRfdsAll, @@ -978,8 +978,15 @@ impl ApiContext { .map_err(|err| err.inner_into())?; let sha = latest_revision.commit_sha; - self.commit_rfd_document(caller, rfd_number, document, message, &sha, branch_name) - .await + self.commit_rfd_document( + caller, + rfd_number.into(), + document, + message, + &sha, + branch_name, + ) + .await } else { resource_restricted() } @@ -989,12 +996,12 @@ impl ApiContext { async fn commit_rfd_document( &self, caller: &ApiCaller, - rfd_number: i32, + rfd_number: RfdNumber, document: &str, message: Option<&str>, head: &str, branch_name: Option<&str>, - ) -> ResourceResult<(), UpdateRfdContentError> { + ) -> ResourceResult, UpdateRfdContentError> { let mut github_locations = self .github .locations_for_commit(head.to_string()) @@ -1014,7 +1021,7 @@ impl ApiContext { 0 => { tracing::warn!( head, - rfd_number, + ?rfd_number, "Failed to find a GitHub location for most recent revision" ); Err(ResourceError::DoesNotExist) @@ -1030,12 +1037,34 @@ impl ApiContext { // Unwrap is checked by the location length let location = github_locations.pop().unwrap(); - location - .upsert(&rfd_number.into(), document.as_bytes(), &message) + let commit = location + .upsert(&rfd_number, document.as_bytes(), &message) .await .map_err(UpdateRfdContentError::GitHub) .to_resource_result()?; - Ok(()) + + // If we committed a change, immediately register a job as well. This may conflict with + // a job already added by a webhook, this is fine and we can ignore the error + if let Some(commit) = &commit { + let new_job = NewJob { + owner: self.github.owner.clone(), + repository: self.github.repo.clone(), + branch: rfd_number.as_number_string(), + sha: commit.to_string(), + rfd: rfd_number.into(), + // This job is not being triggered by a webhook + webhook_delivery_id: None, + committed_at: Utc::now(), // Use the current time or extract from commit if available + }; + + if let Err(err) = self.register_job(new_job).await { + tracing::info!(?err, "Failed to register job for RFD update"); + } else { + tracing::debug!(?rfd_number, ?commit, "Registered job for RFD update"); + } + } + + Ok(commit) } _ => Err(UpdateRfdContentError::InternalState).to_resource_result(), } diff --git a/rfd-github/src/lib.rs b/rfd-github/src/lib.rs index 7f7d3562..53b40508 100644 --- a/rfd-github/src/lib.rs +++ b/rfd-github/src/lib.rs @@ -676,7 +676,7 @@ impl GitHubRfdLocation { rfd_number: &RfdNumber, content: &[u8], message: &str, - ) -> Result<(), GitHubError> { + ) -> Result, GitHubError> { let readme_path = self.readme_path(&self.client, rfd_number).await; // let FetchedRfdContent { decoded, .. } = self // .fetch_content(&self.client, &readme_path, &self.commit) @@ -700,7 +700,7 @@ impl GitHubRfdLocation { // We can short circuit if the new and old content are the same if content == &decoded { tracing::info!("File contents are the same. Skipping commit"); - return Ok(()); + return Ok(None); } tracing::info!( @@ -709,7 +709,8 @@ impl GitHubRfdLocation { "Writing file to GitHub" ); - self.client + let response = self + .client .repos() .create_or_update_file_contents( &self.owner, @@ -726,7 +727,7 @@ impl GitHubRfdLocation { ) .await?; - Ok(()) + Ok(Some(response.body.commit.sha)) } } From e4512c15694240a9fb2a1cc67a7ee8935a87ecd2 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Tue, 30 Apr 2024 14:22:29 -0500 Subject: [PATCH 15/26] Example config and field fixes --- rfd-api/config.example.toml | 2 +- rfd-api/src/server.rs | 3 +++ rfd-api/src/util.rs | 2 +- rfd-processor/config.example.toml | 6 ++++++ rfd-processor/src/updater/mod.rs | 1 + 5 files changed, 12 insertions(+), 2 deletions(-) diff --git a/rfd-api/config.example.toml b/rfd-api/config.example.toml index 6171a84c..e3ad12d9 100644 --- a/rfd-api/config.example.toml +++ b/rfd-api/config.example.toml @@ -98,7 +98,7 @@ template = """""" required_fields = [] # The GitHub repository to use to write RFDs -[source] +[services.github] # GitHub user or organization owner = "" # GitHub repository name diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index dd47866e..853dc0fd 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -191,6 +191,9 @@ pub fn server( .expect("Failed to register endpoint"); if let Some(spec) = config.spec_output { + // TODO: How do we validate that spec_output can be read or written? Currently File::create + // panics if the file path is not a valid path + // Create the API schema. let mut api_definition = &mut api.openapi(spec.title, &""); api_definition = api_definition diff --git a/rfd-api/src/util.rs b/rfd-api/src/util.rs index 46699887..8c3e752c 100644 --- a/rfd-api/src/util.rs +++ b/rfd-api/src/util.rs @@ -57,7 +57,7 @@ pub mod response { } pub fn forbidden() -> HttpError { - client_error(StatusCode::FORBIDDEN, "Unauthorized") + client_error(StatusCode::FORBIDDEN, "Forbidden") } pub fn client_error(status_code: StatusCode, message: S) -> HttpError diff --git a/rfd-processor/config.example.toml b/rfd-processor/config.example.toml index c6961e40..82e959a7 100644 --- a/rfd-processor/config.example.toml +++ b/rfd-processor/config.example.toml @@ -1,3 +1,6 @@ +# Controls if the processor should run +processor_enabled = true + # How many jobs should be processed at once processor_batch_size = 10 @@ -8,6 +11,9 @@ processor_interval = 30 # services or only generate what would be persisted. processor_update_mode = "read" +# Controls if the scanner should run +scanner_enabled = true + # How often the processor scanner should check the remote GitHub repo for RFDs scanner_interval = 900 diff --git a/rfd-processor/src/updater/mod.rs b/rfd-processor/src/updater/mod.rs index 8814eefb..12804cc5 100644 --- a/rfd-processor/src/updater/mod.rs +++ b/rfd-processor/src/updater/mod.rs @@ -322,6 +322,7 @@ pub trait RfdUpdateAction: Debug { pub type BoxedAction = Box; #[derive(Copy, Clone, Debug, Deserialize, PartialEq)] +#[serde(rename_all = "lowercase")] pub enum RfdUpdateMode { Read, Write, From c6c089852bf50b61c3b2e2a1acc0305a1e2cc680 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Tue, 30 Apr 2024 15:53:46 -0500 Subject: [PATCH 16/26] Start on setup doc --- SETUP.md | 92 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) create mode 100644 SETUP.md diff --git a/SETUP.md b/SETUP.md new file mode 100644 index 00000000..cd4320c2 --- /dev/null +++ b/SETUP.md @@ -0,0 +1,92 @@ +# rfd-api + +Backend services and tools for processing and managing RFDs + +## Setup + +To start with there are few dependencies for running the RFD API. Notably the RFD API is broken up +in to two applications, the `rfd-api` and the `rfd-processor`. See [README.md] for more information +on the distinctions. + +### Dependencies + +General + * Rust 1.77 + * Postgres 14 + +Processor + * Asciidoctor 2.0.16 + * Node 20.11.0 + * NPM Packages: + * Ruby 3.0.2 + * Ruby Gems: + * asciidoctor-mermaid 0.4.1 + * asciidoctor-pdf 2.3.10 + * mmdc 10.6.1 + * rouge 4.2.0 + +Optional + * Meilisearch (if search is to be supported) + +### Build + +Run `cargo build --release` to build all of the project binaries. Two binaries will be generated and +emitted at: + +* `target/release/rfd-api` +* `target/release/rfd-processor` + +### Installation + +Once all of the dependencies have been installed, database migrations will need to be run to prepare +the database tables. These can be run using the [`diesel` cli ](https://diesel.rs/guides/getting-started). + +To run them: + +```sh +cd rfd-model +DATABASE_URL= diesel migration run +``` + +Replace `` with the url to the Postgres instance that the API and processor will be +configured to run against. + +### Configuration + +Each part (the api and processor) has its own configuration file, and the respective application +directories have example files called `config.example.toml`. Where possible the define either +default values, or disabled options. + +#### API + +The two sections in the API configuration to pay attention to are the `[[keys]]` and `[[authn]]` +configurations. + +`[[keys]]` define the key pairs used to sign API keys and session tokens. Two sources are supported +for keys, either local or GCP Cloud KMS. Placeholder configurations are provided for both sources as +examples. During local development it is recommended to generate a new RSA key pair locally for use. + +`[[authn]]` is a list of authentication providers that should be enabled. Google and GitHub are the +only authentication providers supported currently and placeholders are available in the API example +configuration. + +For local development remote authentication can be bypassed by using the `local-dev` feature. +Instead of using a remote authentication provider, the API can be run via: + +```sh +cargo run -p rfd-api --features local-dev +``` + +This will run the API with a [`POST /login/local`]() endpoint exposed. This endpoint allows for +logging in with an arbitrary email and user supplied unique identifier. To use this with the CLI, +the `local-dev` feature will need to be passed to the CLI build. + +```sh +cargo run -p rfd-cli --features local-dev +``` + +#### Processor + +The processor has multiple jobs that are able to be run, and configuration is only required for +jobs that are going to be run. The `actions` key defines the jobs that should be run. By default +all jobs are disabled. In this this mode the processor will only construct a database of RFDs. \ No newline at end of file From e4df6dda61cd336b1ba19c4a339eb2542e59b06b Mon Sep 17 00:00:00 2001 From: augustuswm Date: Tue, 30 Apr 2024 15:54:24 -0500 Subject: [PATCH 17/26] Link endpoint --- SETUP.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/SETUP.md b/SETUP.md index cd4320c2..da0be74d 100644 --- a/SETUP.md +++ b/SETUP.md @@ -77,9 +77,10 @@ Instead of using a remote authentication provider, the API can be run via: cargo run -p rfd-api --features local-dev ``` -This will run the API with a [`POST /login/local`]() endpoint exposed. This endpoint allows for -logging in with an arbitrary email and user supplied unique identifier. To use this with the CLI, -the `local-dev` feature will need to be passed to the CLI build. +This will run the API with a [`POST /login/local`](rfd-api/src/endpoints/login/local/mod.rs) endpoint +exposed. This endpoint allows for logging in with an arbitrary email and user supplied unique +identifier. To use this with the CLI, the `local-dev` feature will need to be passed to the CLI +build. ```sh cargo run -p rfd-cli --features local-dev From 8cc8c408100ff10326a1f38556a5e4801d41c34c Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 12:01:20 -0500 Subject: [PATCH 18/26] Extend timeout --- rfd-model/src/storage/postgres.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 805fd421..a47e7217 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -77,7 +77,7 @@ impl PostgresStore { Ok(Self { pool: Pool::builder() - .connection_timeout(Duration::from_secs(15)) + .connection_timeout(Duration::from_secs(30)) .build(manager) .await?, }) From db32c42c5d68d128500a352f002a09d92d447a02 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 12:37:51 -0500 Subject: [PATCH 19/26] Start display_name support for Google --- rfd-api/src/context.rs | 12 ++----- rfd-api/src/endpoints/login/mod.rs | 2 +- rfd-api/src/endpoints/login/oauth/github.rs | 2 +- rfd-api/src/endpoints/login/oauth/google.rs | 37 +++++++++++++++++---- rfd-api/src/mapper/github_username.rs | 21 ++++++++---- 5 files changed, 49 insertions(+), 25 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 6fad135c..4cc0429e 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -1159,12 +1159,7 @@ impl ApiContext { id: Uuid::new_v4(), api_user_id: user.id, emails: info.verified_emails, - // TODO: Refactor in generic display name across providers. This cascades - // into changes needed within mappers - display_names: info - .github_username - .map(|name| vec![name]) - .unwrap_or_default(), + display_names: info.display_name.into_iter().collect::>(), provider: info.external_id.provider().to_string(), provider_id: info.external_id.id().to_string(), }, @@ -1183,10 +1178,7 @@ impl ApiContext { // Update the provider with the newest user info provider.emails = info.verified_emails; - provider.display_names = info - .github_username - .map(|name| vec![name]) - .unwrap_or_default(); + provider.display_names = info.display_name.into_iter().collect::>(); tracing::info!(?provider.id, "Updating provider for user"); diff --git a/rfd-api/src/endpoints/login/mod.rs b/rfd-api/src/endpoints/login/mod.rs index c140e5de..06e270f3 100644 --- a/rfd-api/src/endpoints/login/mod.rs +++ b/rfd-api/src/endpoints/login/mod.rs @@ -146,7 +146,7 @@ impl<'de> Deserialize<'de> for ExternalUserId { pub struct UserInfo { pub external_id: ExternalUserId, pub verified_emails: Vec, - pub github_username: Option, + pub display_name: Option, } #[derive(Debug, Error)] diff --git a/rfd-api/src/endpoints/login/oauth/github.rs b/rfd-api/src/endpoints/login/oauth/github.rs index 9210ccaa..1561de1b 100644 --- a/rfd-api/src/endpoints/login/oauth/github.rs +++ b/rfd-api/src/endpoints/login/oauth/github.rs @@ -92,7 +92,7 @@ impl ExtractUserInfo for GitHubOAuthProvider { Ok(UserInfo { external_id: ExternalUserId::GitHub(user.id.to_string()), verified_emails, - github_username: Some(user.login), + display_name: Some(user.login), }) } } diff --git a/rfd-api/src/endpoints/login/oauth/google.rs b/rfd-api/src/endpoints/login/oauth/google.rs index c459c9ac..c3c77154 100644 --- a/rfd-api/src/endpoints/login/oauth/google.rs +++ b/rfd-api/src/endpoints/login/oauth/google.rs @@ -62,21 +62,46 @@ struct GoogleUserInfo { email_verified: bool, } +#[derive(Debug, Deserialize)] +struct GoogleProfile { + names: Vec, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct GoogleProfileName { + display_name: String, + metadata: GoogleProfileNameMeta, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct GoogleProfileNameMeta { + primary: bool, +} + impl ExtractUserInfo for GoogleOAuthProvider { // There should always be as many entries in the data list as there are endpoints. This should // be changed in the future to be a static check fn extract_user_info(&self, data: &[Bytes]) -> Result { - let remote_info: GoogleUserInfo = serde_json::from_slice(&data[0])?; - let verified_emails = if remote_info.email_verified { - vec![remote_info.email] + let user_info: GoogleUserInfo = serde_json::from_slice(&data[0])?; + let verified_emails = if user_info.email_verified { + vec![user_info.email] } else { vec![] }; + let profile_info: GoogleProfile = serde_json::from_slice(&data[1])?; + let display_name = profile_info + .names + .into_iter() + .filter_map(|name| name.metadata.primary.then(|| name.display_name)) + .nth(0); + Ok(UserInfo { - external_id: ExternalUserId::Google(remote_info.sub), + external_id: ExternalUserId::Google(user_info.sub), verified_emails, - github_username: None, + display_name, }) } } @@ -87,7 +112,7 @@ impl OAuthProvider for GoogleOAuthProvider { } fn scopes(&self) -> Vec<&str> { - vec!["openid", "email"] + vec!["openid", "email", "profile"] } fn client(&self) -> &reqwest::Client { diff --git a/rfd-api/src/mapper/github_username.rs b/rfd-api/src/mapper/github_username.rs index aeb05f32..853d0a8d 100644 --- a/rfd-api/src/mapper/github_username.rs +++ b/rfd-api/src/mapper/github_username.rs @@ -11,7 +11,10 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::{ - context::ApiContext, endpoints::login::UserInfo, util::response::ResourceResult, ApiPermissions, + context::ApiContext, + endpoints::login::{ExternalUserId, UserInfo}, + util::response::ResourceResult, + ApiPermissions, }; use super::MapperRule; @@ -25,6 +28,14 @@ pub struct GitHubUsernameMapper { groups: Vec, } +fn github_username(user: &UserInfo) -> Option<&str> { + match user.external_id { + // Endure that this is GitHub provided user data + ExternalUserId::GitHub(_) => user.display_name.as_deref(), + _ => None, + } +} + #[async_trait] impl MapperRule for GitHubUsernameMapper { async fn permissions_for( @@ -32,9 +43,7 @@ impl MapperRule for GitHubUsernameMapper { _ctx: &ApiContext, user: &UserInfo, ) -> Result { - if user - .github_username - .as_ref() + if github_username(user) .map(|u| u == &self.github_username) .unwrap_or(false) { @@ -49,9 +58,7 @@ impl MapperRule for GitHubUsernameMapper { ctx: &ApiContext, user: &UserInfo, ) -> ResourceResult, StoreError> { - if user - .github_username - .as_ref() + if github_username(user) .map(|u| u == &self.github_username) .unwrap_or(false) { From 8ffa07d0cc31a4da046715dfc2d57ff34e884c5e Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 13:56:40 -0500 Subject: [PATCH 20/26] Fix primary field --- rfd-api/src/endpoints/login/oauth/google.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rfd-api/src/endpoints/login/oauth/google.rs b/rfd-api/src/endpoints/login/oauth/google.rs index c3c77154..957cf451 100644 --- a/rfd-api/src/endpoints/login/oauth/google.rs +++ b/rfd-api/src/endpoints/login/oauth/google.rs @@ -77,6 +77,7 @@ struct GoogleProfileName { #[derive(Debug, Deserialize)] #[serde(rename_all = "camelCase")] struct GoogleProfileNameMeta { + #[serde(default)] primary: bool, } From 3adcc27a6ef121e8478556385b1d35c91ee0ed6a Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 14:31:22 -0500 Subject: [PATCH 21/26] Use display names as opposed to caller ids in commit messages --- rfd-api/src/context.rs | 31 +++++++++++++++++++++++++++++-- rfd-model/src/lib.rs | 1 + rfd-model/src/storage/mod.rs | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 4cc0429e..15703d63 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -43,6 +43,7 @@ use rsa::{ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::{ + cmp::Ordering, collections::{BTreeSet, HashMap}, ops::Add, sync::Arc, @@ -976,8 +977,9 @@ impl ApiContext { .get_latest_rfd_revision(caller, rfd_number) .await .map_err(|err| err.inner_into())?; - let sha = latest_revision.commit_sha; + + self.commit_rfd_document( caller, rfd_number.into(), @@ -1017,6 +1019,31 @@ impl ApiContext { }) .collect::>(); + let mut providers = self + .list_api_user_provider( + caller, + ApiUserProviderFilter::default().api_user_id(Some(vec![caller.id])), + &ListPagination::default(), + ) + .await + .map_err(|err| err.inner_into())?; + + // Prefer a GitHub identity provider, but we will use Google if we can not find one + providers.sort_by(|a, b| { + if a.provider == b.provider { + Ordering::Equal + } else if a.provider == "github" { + Ordering::Less + } else { + Ordering::Greater + } + }); + + let display_name = providers + .get(0) + .and_then(|p| p.display_names.get(0).map(|s| s.clone())) + .unwrap_or_else(|| caller.id.to_string()); + match github_locations.len() { 0 => { tracing::warn!( @@ -1032,7 +1059,7 @@ impl ApiContext { let message = format!( "{}\n\nSubmitted by {}", message.unwrap_or("RFD API update"), - caller.id + display_name, ); // Unwrap is checked by the location length diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index 3f11712e..eaa69fc6 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -190,6 +190,7 @@ pub struct ApiUser { pub struct ApiUserProvider { pub id: Uuid, pub api_user_id: Uuid, + // TODO: This really needs to be an enum, pub provider: String, pub provider_id: String, pub emails: Vec, diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index 68a5bd23..0469ff74 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -323,6 +323,38 @@ pub struct ApiUserProviderFilter { pub deleted: bool, } +impl ApiUserProviderFilter { + pub fn id(mut self, id: Option>) -> Self { + self.id = id; + self + } + + pub fn api_user_id(mut self, api_user_id: Option>) -> Self { + self.api_user_id = api_user_id; + self + } + + pub fn provider(mut self, provider: Option>) -> Self { + self.provider = provider; + self + } + + pub fn provider_id(mut self, provider_id: Option>) -> Self { + self.provider_id = provider_id; + self + } + + pub fn email(mut self, email: Option>) -> Self { + self.email = email; + self + } + + pub fn deleted(mut self, deleted: bool) -> Self { + self.deleted = deleted; + self + } +} + #[cfg_attr(feature = "mock", automock)] #[async_trait] pub trait ApiUserProviderStore { From 31929017f6a91fadc0816b68e31c1727bbfc3011 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 15:22:08 -0500 Subject: [PATCH 22/26] More Google auth fixes --- rfd-api/src/context.rs | 5 ++++- rfd-api/src/endpoints/login/oauth/google.rs | 5 ++++- rfd-api/src/endpoints/rfd.rs | 2 ++ 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 15703d63..ed4ba8da 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -979,6 +979,7 @@ impl ApiContext { .map_err(|err| err.inner_into())?; let sha = latest_revision.commit_sha; + tracing::info!(?sha, "Found commit to update from"); self.commit_rfd_document( caller, @@ -994,7 +995,7 @@ impl ApiContext { } } - #[instrument(skip(self, caller, document))] + #[instrument(skip(self, caller, document), err(Debug))] async fn commit_rfd_document( &self, caller: &ApiCaller, @@ -1004,6 +1005,8 @@ impl ApiContext { head: &str, branch_name: Option<&str>, ) -> ResourceResult, UpdateRfdContentError> { + tracing::info!("Pushing update to GitHub"); + let mut github_locations = self .github .locations_for_commit(head.to_string()) diff --git a/rfd-api/src/endpoints/login/oauth/google.rs b/rfd-api/src/endpoints/login/oauth/google.rs index 957cf451..902fc866 100644 --- a/rfd-api/src/endpoints/login/oauth/google.rs +++ b/rfd-api/src/endpoints/login/oauth/google.rs @@ -141,7 +141,10 @@ impl OAuthProvider for GoogleOAuthProvider { } fn user_info_endpoints(&self) -> Vec<&str> { - vec!["https://openidconnect.googleapis.com/v1/userinfo"] + vec![ + "https://openidconnect.googleapis.com/v1/userinfo", + "https://people.googleapis.com/v1/people/me?personFields=names", + ] } fn device_code_endpoint(&self) -> &str { diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 57e9dcb6..67e18f23 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -382,6 +382,8 @@ async fn set_rfd_attr_op( } }; + tracing::info!("Updated attribute in RFD document"); + // Persist the data back to GitHub. Note that we do not store this back to the database. // We rely on GitHub as the source of truth and revisions are required to tbe linked to // commits From ff15e83ade9e9f9be5986ab026172952e983a1b7 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 16:38:54 -0500 Subject: [PATCH 23/26] More sha typing --- rfd-api-spec.json | 14 +++- rfd-api/src/context.rs | 28 +++---- rfd-cli/src/printer/tab.rs | 8 +- rfd-github/src/lib.rs | 8 +- rfd-sdk/src/generated/sdk.rs | 156 +++++++++++++++++++++++++++++++---- 5 files changed, 172 insertions(+), 42 deletions(-) diff --git a/rfd-api-spec.json b/rfd-api-spec.json index a3a6b68f..ba21a1ab 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -3760,6 +3760,9 @@ "updated_at" ] }, + "CommitSha": { + "type": "string" + }, "ContentFormat": { "type": "string", "enum": [ @@ -3806,6 +3809,9 @@ "request_id" ] }, + "FileSha": { + "type": "string" + }, "FormattedSearchResultHit": { "type": "object", "properties": { @@ -3863,7 +3869,7 @@ "type": "string" }, "commit": { - "type": "string" + "$ref": "#/components/schemas/CommitSha" }, "committed_at": { "type": "string", @@ -3902,7 +3908,7 @@ "format": "int32" }, "sha": { - "type": "string" + "$ref": "#/components/schemas/FileSha" }, "state": { "nullable": true, @@ -4217,7 +4223,7 @@ "type": "string" }, "commit": { - "type": "string" + "$ref": "#/components/schemas/CommitSha" }, "committed_at": { "type": "string", @@ -4247,7 +4253,7 @@ "format": "int32" }, "sha": { - "type": "string" + "$ref": "#/components/schemas/FileSha" }, "state": { "nullable": true, diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 4ae7a05b..ccf86a23 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -735,7 +735,7 @@ impl ApiContext { .map_err(UpdateRfdContentError::InvalidTemplate) .to_resource_result()?; - tracing::info!(?next_rfd_number, commit, "Creating new RFD branch"); + tracing::info!(?next_rfd_number, ?commit, "Creating new RFD branch"); // Branch off of the default branch with a new branch with the padded form of the RFD number self.github @@ -755,7 +755,7 @@ impl ApiContext { next_rfd_number.into(), &content.render(), Some("Reserving RFD number"), - &commit, + commit, Some(&next_rfd_number.as_number_string()), ) .await?; @@ -932,7 +932,7 @@ impl ApiContext { content: &str, message: Option<&str>, branch_name: Option<&str>, - ) -> ResourceResult, UpdateRfdContentError> { + ) -> ResourceResult, UpdateRfdContentError> { if caller.any(&[ &ApiPermission::UpdateRfd(rfd_number), &ApiPermission::UpdateRfdsAll, @@ -942,7 +942,7 @@ impl ApiContext { .await .map_err(|err| err.inner_into())?; - let sha = latest_revision.commit_sha.clone(); + let sha = latest_revision.commit.clone(); let mut updated_content: RfdContent = latest_revision.into(); updated_content.update_body(content); @@ -951,7 +951,7 @@ impl ApiContext { rfd_number.into(), updated_content.raw(), message, - &sha, + sha, branch_name, ) .await @@ -968,7 +968,7 @@ impl ApiContext { document: &str, message: Option<&str>, branch_name: Option<&str>, - ) -> ResourceResult, UpdateRfdContentError> { + ) -> ResourceResult, UpdateRfdContentError> { if caller.any(&[ &ApiPermission::UpdateRfd(rfd_number), &ApiPermission::UpdateRfdsAll, @@ -977,7 +977,7 @@ impl ApiContext { .get_latest_rfd_revision(caller, rfd_number) .await .map_err(|err| err.inner_into())?; - let sha = latest_revision.commit_sha; + let sha = latest_revision.commit; tracing::info!(?sha, "Found commit to update from"); @@ -986,7 +986,7 @@ impl ApiContext { rfd_number.into(), document, message, - &sha, + sha, branch_name, ) .await @@ -1002,14 +1002,14 @@ impl ApiContext { rfd_number: RfdNumber, document: &str, message: Option<&str>, - head: &str, + head: CommitSha, branch_name: Option<&str>, - ) -> ResourceResult, UpdateRfdContentError> { + ) -> ResourceResult, UpdateRfdContentError> { tracing::info!("Pushing update to GitHub"); let mut github_locations = self .github - .locations_for_commit(head.to_string()) + .locations_for_commit(head.clone()) .await .map_err(UpdateRfdContentError::GitHub) .to_resource_result()? @@ -1050,7 +1050,7 @@ impl ApiContext { match github_locations.len() { 0 => { tracing::warn!( - head, + ?head, ?rfd_number, "Failed to find a GitHub location for most recent revision" ); @@ -1075,12 +1075,12 @@ impl ApiContext { // If we committed a change, immediately register a job as well. This may conflict with // a job already added by a webhook, this is fine and we can ignore the error - if let Some(commit) = &commit { + if let Some(commit) = commit.clone() { let new_job = NewJob { owner: self.github.owner.clone(), repository: self.github.repo.clone(), branch: rfd_number.as_number_string(), - sha: commit.to_string(), + sha: commit.clone(), rfd: rfd_number.into(), // This job is not being triggered by a webhook webhook_delivery_id: None, diff --git a/rfd-cli/src/printer/tab.rs b/rfd-cli/src/printer/tab.rs index 98de7d2b..df55fe8d 100644 --- a/rfd-cli/src/printer/tab.rs +++ b/rfd-cli/src/printer/tab.rs @@ -431,8 +431,8 @@ impl TabDisplay for ListRfd { "discussion", &self.discussion.as_ref().map(|s| s.as_str()).unwrap_or(""), ); - printer.print_field(tw, level, "sha", &self.sha); - printer.print_field(tw, level, "commit", &self.commit); + printer.print_field(tw, level, "sha", &self.sha.as_str()); + printer.print_field(tw, level, "commit", &self.commit.as_str()); printer.print_field(tw, level, "committed_at", &self.committed_at); } } @@ -483,8 +483,8 @@ impl TabDisplay for FullRfd { ); printer.print_field(tw, level, "pdfs", &""); self.pdfs.display(tw, level + 1, printer); - printer.print_field(tw, level, "sha", &self.sha); - printer.print_field(tw, level, "commit", &self.commit); + printer.print_field(tw, level, "sha", &self.sha.as_str()); + printer.print_field(tw, level, "commit", &self.commit.as_str()); printer.print_field(tw, level, "committed_at", &self.committed_at); writeln!(tw, ""); writeln!(tw, "{}", self.content); diff --git a/rfd-github/src/lib.rs b/rfd-github/src/lib.rs index 034274fb..542595f4 100644 --- a/rfd-github/src/lib.rs +++ b/rfd-github/src/lib.rs @@ -152,7 +152,7 @@ impl GitHubRfdRepo { Ok(GitHubNewRfdNumber { number: next_rfd_number, - commit: default.commit.sha, + commit: default.commit.sha.into(), }) } @@ -679,7 +679,7 @@ impl GitHubRfdLocation { rfd_number: &RfdNumber, content: &[u8], message: &str, - ) -> Result, GitHubError> { + ) -> Result, GitHubError> { let readme_path = self.readme_path(&self.client, rfd_number).await; let (decoded, sha) = match self .fetch_content(&self.client, &readme_path, &self.commit) @@ -726,7 +726,7 @@ impl GitHubRfdLocation { ) .await?; - Ok(Some(response.body.commit.sha)) + Ok(Some(response.body.commit.sha.into())) } } @@ -762,7 +762,7 @@ pub struct GitHubRfdUpdate { #[derive(Debug, Clone)] pub struct GitHubNewRfdNumber { pub number: RfdNumber, - pub commit: String, + pub commit: CommitSha, } // TODO: Expand this diff --git a/rfd-sdk/src/generated/sdk.rs b/rfd-sdk/src/generated/sdk.rs index ad963594..894973b3 100644 --- a/rfd-sdk/src/generated/sdk.rs +++ b/rfd-sdk/src/generated/sdk.rs @@ -2781,6 +2781,68 @@ pub mod types { } } + /// CommitSha + /// + ///
JSON schema + /// + /// ```json + /// { + /// "type": "string" + /// } + + /// ``` + ///
+ #[derive( + Clone, + Debug, + Deserialize, + Eq, + Hash, + Ord, + PartialEq, + PartialOrd, + Serialize, + schemars :: JsonSchema, + )] + pub struct CommitSha(pub String); + impl std::ops::Deref for CommitSha { + type Target = String; + fn deref(&self) -> &String { + &self.0 + } + } + + impl From for String { + fn from(value: CommitSha) -> Self { + value.0 + } + } + + impl From<&CommitSha> for CommitSha { + fn from(value: &CommitSha) -> Self { + value.clone() + } + } + + impl From for CommitSha { + fn from(value: String) -> Self { + Self(value) + } + } + + impl std::str::FromStr for CommitSha { + type Err = std::convert::Infallible; + fn from_str(value: &str) -> Result { + Ok(Self(value.to_string())) + } + } + + impl ToString for CommitSha { + fn to_string(&self) -> String { + self.0.to_string() + } + } + /// ContentFormat /// ///
JSON schema @@ -2964,6 +3026,68 @@ pub mod types { } } + /// FileSha + /// + ///
JSON schema + /// + /// ```json + /// { + /// "type": "string" + /// } + + /// ``` + ///
+ #[derive( + Clone, + Debug, + Deserialize, + Eq, + Hash, + Ord, + PartialEq, + PartialOrd, + Serialize, + schemars :: JsonSchema, + )] + pub struct FileSha(pub String); + impl std::ops::Deref for FileSha { + type Target = String; + fn deref(&self) -> &String { + &self.0 + } + } + + impl From for String { + fn from(value: FileSha) -> Self { + value.0 + } + } + + impl From<&FileSha> for FileSha { + fn from(value: &FileSha) -> Self { + value.clone() + } + } + + impl From for FileSha { + fn from(value: String) -> Self { + Self(value) + } + } + + impl std::str::FromStr for FileSha { + type Err = std::convert::Infallible; + fn from_str(value: &str) -> Result { + Ok(Self(value.to_string())) + } + } + + impl ToString for FileSha { + fn to_string(&self) -> String { + self.0.to_string() + } + } + /// FormattedSearchResultHit /// ///
JSON schema @@ -3086,7 +3210,7 @@ pub mod types { /// ] /// }, /// "commit": { - /// "type": "string" + /// "$ref": "#/components/schemas/CommitSha" /// }, /// "committed_at": { /// "type": "string", @@ -3132,7 +3256,7 @@ pub mod types { /// "format": "int32" /// }, /// "sha": { - /// "type": "string" + /// "$ref": "#/components/schemas/FileSha" /// }, /// "state": { /// "type": [ @@ -3157,7 +3281,7 @@ pub mod types { pub struct FullRfd { #[serde(default, skip_serializing_if = "Option::is_none")] pub authors: Option, - pub commit: String, + pub commit: CommitSha, pub committed_at: chrono::DateTime, pub content: String, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -3170,7 +3294,7 @@ pub mod types { pub link: Option, pub pdfs: Vec, pub rfd_number: i32, - pub sha: String, + pub sha: FileSha, #[serde(default, skip_serializing_if = "Option::is_none")] pub state: Option, pub title: String, @@ -3859,7 +3983,7 @@ pub mod types { /// ] /// }, /// "commit": { - /// "type": "string" + /// "$ref": "#/components/schemas/CommitSha" /// }, /// "committed_at": { /// "type": "string", @@ -3895,7 +4019,7 @@ pub mod types { /// "format": "int32" /// }, /// "sha": { - /// "type": "string" + /// "$ref": "#/components/schemas/FileSha" /// }, /// "state": { /// "type": [ @@ -3920,7 +4044,7 @@ pub mod types { pub struct ListRfd { #[serde(default, skip_serializing_if = "Option::is_none")] pub authors: Option, - pub commit: String, + pub commit: CommitSha, pub committed_at: chrono::DateTime, #[serde(default, skip_serializing_if = "Option::is_none")] pub discussion: Option, @@ -3931,7 +4055,7 @@ pub mod types { #[serde(default, skip_serializing_if = "Option::is_none")] pub link: Option, pub rfd_number: i32, - pub sha: String, + pub sha: FileSha, #[serde(default, skip_serializing_if = "Option::is_none")] pub state: Option, pub title: String, @@ -6880,7 +7004,7 @@ pub mod types { #[derive(Clone, Debug)] pub struct FullRfd { authors: Result, String>, - commit: Result, + commit: Result, committed_at: Result, String>, content: Result, discussion: Result, String>, @@ -6890,7 +7014,7 @@ pub mod types { link: Result, String>, pdfs: Result, String>, rfd_number: Result, - sha: Result, + sha: Result, state: Result, String>, title: Result, visibility: Result, @@ -6931,7 +7055,7 @@ pub mod types { } pub fn commit(mut self, value: T) -> Self where - T: std::convert::TryInto, + T: std::convert::TryInto, T::Error: std::fmt::Display, { self.commit = value @@ -7031,7 +7155,7 @@ pub mod types { } pub fn sha(mut self, value: T) -> Self where - T: std::convert::TryInto, + T: std::convert::TryInto, T::Error: std::fmt::Display, { self.sha = value @@ -8033,7 +8157,7 @@ pub mod types { #[derive(Clone, Debug)] pub struct ListRfd { authors: Result, String>, - commit: Result, + commit: Result, committed_at: Result, String>, discussion: Result, String>, format: Result, @@ -8041,7 +8165,7 @@ pub mod types { labels: Result, String>, link: Result, String>, rfd_number: Result, - sha: Result, + sha: Result, state: Result, String>, title: Result, visibility: Result, @@ -8080,7 +8204,7 @@ pub mod types { } pub fn commit(mut self, value: T) -> Self where - T: std::convert::TryInto, + T: std::convert::TryInto, T::Error: std::fmt::Display, { self.commit = value @@ -8160,7 +8284,7 @@ pub mod types { } pub fn sha(mut self, value: T) -> Self where - T: std::convert::TryInto, + T: std::convert::TryInto, T::Error: std::fmt::Display, { self.sha = value From d7316d7bdcf4f294431b7fef30e69fbe57a4ce17 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Mon, 13 May 2024 16:53:19 -0500 Subject: [PATCH 24/26] Filter out providers that do not have a display name --- rfd-api/src/context.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index ccf86a23..e3f0be8e 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -1043,7 +1043,9 @@ impl ApiContext { }); let display_name = providers - .get(0) + .into_iter() + .filter(|provider| !provider.display_names.is_empty()) + .nth(0) .and_then(|p| p.display_names.get(0).map(|s| s.clone())) .unwrap_or_else(|| caller.id.to_string()); From acbb74b8f20496ec76206745f21fb12186372d23 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Tue, 14 May 2024 09:11:00 -0500 Subject: [PATCH 25/26] Update local dev feature --- rfd-api/src/endpoints/login/local/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfd-api/src/endpoints/login/local/mod.rs b/rfd-api/src/endpoints/login/local/mod.rs index 94850929..f2095f6a 100644 --- a/rfd-api/src/endpoints/login/local/mod.rs +++ b/rfd-api/src/endpoints/login/local/mod.rs @@ -39,7 +39,7 @@ pub async fn local_login( let info = UserInfo { external_id: super::ExternalUserId::Local(body.external_id), verified_emails: vec![body.email], - github_username: None, + display_name: Some("Local Dev".to_string()), }; let (api_user, api_user_provider) = ctx From b287ba254fb8bcc8409a0a14c28ef7605c225875 Mon Sep 17 00:00:00 2001 From: augustuswm Date: Tue, 14 May 2024 09:36:29 -0500 Subject: [PATCH 26/26] Fix Google redirect test --- rfd-api/src/endpoints/login/oauth/code.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rfd-api/src/endpoints/login/oauth/code.rs b/rfd-api/src/endpoints/login/oauth/code.rs index 25617926..694af23d 100644 --- a/rfd-api/src/endpoints/login/oauth/code.rs +++ b/rfd-api/src/endpoints/login/oauth/code.rs @@ -850,7 +850,7 @@ mod tests { ) .unwrap(); - let expected_location = format!("https://accounts.google.com/o/oauth2/v2/auth?response_type=code&client_id=google_web_client_id&state={}&code_challenge={}&code_challenge_method=S256&redirect_uri=https%3A%2F%2Fapi.oxeng.dev%2Flogin%2Foauth%2Fgoogle%2Fcode%2Fcallback&scope=openid+email", attempt.id, challenge.as_str()); + let expected_location = format!("https://accounts.google.com/o/oauth2/v2/auth?response_type=code&client_id=google_web_client_id&state={}&code_challenge={}&code_challenge_method=S256&redirect_uri=https%3A%2F%2Fapi.oxeng.dev%2Flogin%2Foauth%2Fgoogle%2Fcode%2Fcallback&scope=openid+email+profile", attempt.id, challenge.as_str()); assert_eq!( expected_location,