From b2fdb217339585d2c6fdd5267dfe723ec719671f Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 08:57:53 -0600 Subject: [PATCH 01/17] Work on refactoring storage an retrieval --- Cargo.lock | 2 + rfd-api/Cargo.toml | 2 + rfd-api/src/context.rs | 515 +++++++----------------------- rfd-api/src/endpoints/meta.rs | 89 ++++-- rfd-api/src/endpoints/rfd.rs | 145 ++++++--- rfd-api/src/main.rs | 7 +- rfd-api/src/server.rs | 15 +- rfd-cli/src/main.rs | 2 +- rfd-github/src/lib.rs | 22 ++ rfd-model/src/db.rs | 6 +- rfd-model/src/lib.rs | 38 ++- rfd-model/src/storage/mock.rs | 256 +++++++++++++++ rfd-model/src/storage/mod.rs | 66 +++- rfd-model/src/storage/postgres.rs | 189 ++++++++--- rfd-processor/src/context.rs | 3 +- 15 files changed, 826 insertions(+), 531 deletions(-) create mode 100644 rfd-model/src/storage/mock.rs diff --git a/Cargo.lock b/Cargo.lock index 576f533b..725a96d8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2852,8 +2852,10 @@ name = "rfd-api" version = "0.8.0" dependencies = [ "anyhow", + "async-bb8-diesel", "async-trait", "base64 0.22.1", + "bb8", "chrono", "config", "cookie", diff --git a/rfd-api/Cargo.toml b/rfd-api/Cargo.toml index 7fb8c274..074539ac 100644 --- a/rfd-api/Cargo.toml +++ b/rfd-api/Cargo.toml @@ -9,8 +9,10 @@ local-dev = ["v-api/local-dev"] [dependencies] anyhow = { workspace = true } +async-bb8-diesel = { workspace = true } async-trait = { workspace = true } base64 = { workspace = true } +bb8 = { workspace = true } chrono = { workspace = true, features = ["serde"] } config = { workspace = true } cookie = { workspace = true } diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 91e3e187..d1c64ff9 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -18,10 +18,10 @@ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, storage::{ - JobStore, RfdFilter, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, - RfdRevisionStore, RfdStore, + JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, + RfdRevisionMetaStore, RfdStorage, RfdStore, }, - CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdRevision, + CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdRevision, }; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, @@ -51,25 +51,9 @@ use crate::{ static UNLIMITED: i64 = 9999999; -pub trait Storage: - RfdStore + RfdRevisionStore + RfdPdfStore + RfdRevisionMetaStore + JobStore + Send + Sync + 'static -{ -} -impl Storage for T where - T: RfdStore - + RfdRevisionStore - + RfdPdfStore - + RfdRevisionMetaStore - + JobStore - + Send - + Sync - + 'static -{ -} - pub struct RfdContext { pub public_url: String, - pub storage: Arc, + pub storage: Arc, pub search: SearchContext, pub content: ContentContext, pub github: GitHubRfdRepo, @@ -107,9 +91,9 @@ pub enum UpdateRfdContentError { Storage(#[from] StoreError), } -#[partial(RfdMeta)] +#[partial(RfdWithoutContent)] #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] -pub struct FullRfd { +pub struct RfdWithContent { pub id: TypedUuid, pub rfd_number: i32, pub link: Option, @@ -118,19 +102,19 @@ pub struct FullRfd { pub state: Option, pub authors: Option, pub labels: Option, - #[partial(RfdMeta(skip))] + #[partial(RfdWithoutContent(skip))] pub content: String, pub format: ContentFormat, pub sha: FileSha, pub commit: CommitSha, pub committed_at: DateTime, - #[partial(RfdMeta(skip))] - pub pdfs: Vec, + #[partial(RfdWithoutContent(skip))] + pub pdfs: Vec, pub visibility: Visibility, } #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] -pub struct FullRfdPdfEntry { +pub struct PdfEntry { pub source: String, pub link: String, } @@ -138,7 +122,7 @@ pub struct FullRfdPdfEntry { impl RfdContext { pub async fn new( public_url: String, - storage: Arc, + storage: Arc, search: SearchConfig, content: ContentConfig, services: ServicesConfig, @@ -220,11 +204,11 @@ impl RfdContext { &self, caller: &Caller, filter: Option, - ) -> ResourceResult, StoreError> { + ) -> ResourceResult, StoreError> { // List all of the RFDs first and then perform filter. This should be be improved once // filters can be combined to support OR expressions. Effectively we need to be able to // express "Has access to OR is public" with a filter - let mut rfds = RfdStore::list( + let mut rfds = RfdMetaStore::list( &*self.storage, filter.unwrap_or_default(), &ListPagination::default().limit(UNLIMITED), @@ -268,7 +252,7 @@ impl RfdContext { let mut rfd_list = rfds .into_iter() .zip(rfd_revisions) - .map(|(rfd, revision)| RfdMeta { + .map(|(rfd, revision)| RfdWithoutContent { id: rfd.id, rfd_number: rfd.rfd_number, link: rfd.link, @@ -369,180 +353,152 @@ impl RfdContext { } #[instrument(skip(self, caller))] - pub async fn get_rfd( + async fn get_rfd_by_number( &self, caller: &Caller, rfd_number: i32, sha: Option, - ) -> ResourceResult { - // list_rfds performs authorization checks, if the caller does not have access to the - // requested RFD an empty Vec will be returned - let rfds = self - .list_rfds( - caller, - Some(RfdFilter::default().rfd_number(Some(vec![rfd_number]))), - ) - .await?; - - if let Some(rfd) = rfds.into_iter().nth(0) { - // 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( - &*self.storage, - RfdRevisionFilter::default() - .rfd(Some(vec![rfd.id])) - .sha(sha.map(|sha| vec![sha])), - &ListPagination::default().limit(1), - ) - .await - .to_resource_result()?; - - if let Some(revision) = latest_revision.into_iter().nth(0) { - let pdfs = RfdPdfStore::list( - &*self.storage, - RfdPdfFilter::default().rfd_revision(Some(vec![revision.id])), - &ListPagination::default(), - ) - .await - .to_resource_result()?; + ) -> ResourceResult { + let rfd = RfdStore::list( + &*self.storage, + RfdFilter::default() + .rfd_number(Some(vec![rfd_number])) + .sha(sha.map(|sha| vec![sha])), + &ListPagination::latest(), + ) + .await + .to_resource_result()? + .pop(); - Ok(FullRfd { - id: rfd.id, - rfd_number: rfd.rfd_number, - link: rfd.link, - discussion: revision.discussion, - title: revision.title, - state: revision.state, - authors: revision.authors, - labels: revision.labels, - content: revision.content, - format: revision.content_format, - sha: revision.sha, - commit: revision.commit.into(), - committed_at: revision.committed_at, - pdfs: pdfs - .into_iter() - .map(|pdf| FullRfdPdfEntry { - source: pdf.source.to_string(), - link: pdf.link, - }) - .collect(), - visibility: rfd.visibility, - }) + if let Some(rfd) = rfd { + if caller.can(&RfdPermission::GetRfdsAll) + || caller.can(&RfdPermission::GetRfd(rfd.rfd_number)) + || rfd.visibility == Visibility::Public + { + Ok(rfd) } else { - // It should not be possible to reach this branch. If we have then the database - // has entered an inconsistent state - tracing::error!("Looking up revision for RFD returned no results"); resource_not_found() } } else { - // Either the RFD does not exist, or the caller is not allowed to access it resource_not_found() } } #[instrument(skip(self, caller))] - pub async fn get_rfd_meta( + pub async fn view_rfd( &self, caller: &Caller, rfd_number: i32, sha: Option, - ) -> ResourceResult { - // list_rfds performs authorization checks, if the caller does not have access to the - // requested RFD an empty Vec will be returned - let rfds = self - .list_rfds( - caller, - Some(RfdFilter::default().rfd_number(Some(vec![rfd_number]))), - ) - .await?; + ) -> ResourceResult { + let rfd = self.get_rfd_by_number(caller, rfd_number, sha).await?; + let pdfs = RfdPdfStore::list( + &*self.storage, + RfdPdfFilter::default().rfd_revision(Some(vec![rfd.content.id])), + &ListPagination::default(), + ) + .await + .to_resource_result()?; - if let Some(rfd) = rfds.into_iter().nth(0) { - Ok(rfd) - } else { - // Either the RFD does not exist, or the caller is not allowed to access it - resource_not_found() - } + Ok(RfdWithContent { + id: rfd.id, + rfd_number: rfd.rfd_number, + link: rfd.link, + discussion: rfd.content.discussion, + title: rfd.content.title, + state: rfd.content.state, + authors: rfd.content.authors, + labels: rfd.content.labels, + content: rfd.content.content, + format: rfd.content.content_format, + sha: rfd.content.sha, + commit: rfd.content.commit.into(), + committed_at: rfd.content.committed_at, + pdfs: pdfs + .into_iter() + .map(|pdf| PdfEntry { + source: pdf.source.to_string(), + link: pdf.link, + }) + .collect(), + visibility: rfd.visibility, + }) } #[instrument(skip(self, caller))] - pub async fn get_rfd_revision( + async fn get_rfd_meta_by_number( &self, caller: &Caller, rfd_number: i32, sha: Option, - ) -> ResourceResult { - if caller.any(&[ - &RfdPermission::GetRfd(rfd_number), - &RfdPermission::GetRfdsAll, - ]) { - let rfds = RfdStore::list( - &*self.storage, - RfdFilter::default().rfd_number(Some(vec![rfd_number])), - &ListPagination::default().limit(1), - ) - .await - .to_resource_result()?; - if let Some(rfd) = rfds.into_iter().nth(0) { - let latest_revision = RfdRevisionStore::list( - &*self.storage, - RfdRevisionFilter::default() - .rfd(Some(vec![rfd.id])) - .sha(sha.map(|sha| vec![sha])), - &ListPagination::default().limit(1), - ) - .await - .to_resource_result()?; + ) -> ResourceResult { + let rfd = RfdMetaStore::list( + &*self.storage, + RfdFilter::default() + .rfd_number(Some(vec![rfd_number])) + .sha(sha.map(|sha| vec![sha])), + &ListPagination::latest(), + ) + .await + .to_resource_result()? + .pop(); - match latest_revision.into_iter().nth(0) { - Some(revision) => Ok(revision), - None => resource_not_found(), - } + if let Some(rfd) = rfd { + if caller.can(&RfdPermission::GetRfdsAll) + || caller.can(&RfdPermission::GetRfd(rfd.rfd_number)) + || rfd.visibility == Visibility::Public + { + Ok(rfd) } else { resource_not_found() } } else { - resource_restricted() + resource_not_found() } } - async fn get_latest_rfd_revision( + #[instrument(skip(self, caller))] + pub async fn view_rfd_meta( &self, caller: &Caller, rfd_number: i32, - ) -> ResourceResult { - if caller.any(&[ - &RfdPermission::GetRfd(rfd_number), - &RfdPermission::GetRfdsAll, - ]) { - let rfds = RfdStore::list( - &*self.storage, - RfdFilter::default().rfd_number(Some(vec![rfd_number])), - &ListPagination::default().limit(1), - ) - .await - .to_resource_result()?; + sha: Option, + ) -> ResourceResult { + let rfd = self.get_rfd_meta_by_number(caller, rfd_number, sha).await?; + Ok(RfdWithoutContent { + id: rfd.id, + rfd_number: rfd.rfd_number, + link: rfd.link, + discussion: rfd.content.discussion, + title: rfd.content.title, + state: rfd.content.state, + authors: rfd.content.authors, + labels: rfd.content.labels, + format: rfd.content.content_format, + sha: rfd.content.sha, + commit: rfd.content.commit.into(), + committed_at: rfd.content.committed_at, + visibility: rfd.visibility, + }) + } - if let Some(rfd) = rfds.into_iter().nth(0) { - let revisions = RfdRevisionStore::list( - &*self.storage, - RfdRevisionFilter::default().rfd(Some(vec![rfd.id])), - &ListPagination::default().limit(1), - ) - .await - .to_resource_result()?; + #[instrument(skip(self, caller))] + pub async fn view_rfd_revision( + &self, + caller: &Caller, + rfd_number: i32, + sha: Option, + ) -> ResourceResult { + let rfd = self.get_rfd_by_number(caller, rfd_number, sha).await?; + Ok(rfd.content) + } - match revisions.into_iter().nth(0) { - Some(revision) => Ok(revision), - None => resource_not_found(), - } - } else { - resource_not_found() - } - } else { - resource_restricted() - } + async fn get_latest_rfd_revision( + &self, + caller: &Caller, + rfd_number: i32, + ) -> ResourceResult { + self.view_rfd_revision(caller, rfd_number, None).await } #[instrument(skip(self, caller, content))] @@ -767,18 +723,9 @@ impl RfdContext { #[cfg(test)] pub(crate) mod test_mocks { - use async_trait::async_trait; - use newtype_uuid::TypedUuid; use rand::RngCore; use rfd_data::content::RfdTemplate; - use rfd_model::{ - storage::{ - JobStore, MockJobStore, MockRfdPdfStore, MockRfdRevisionMetaStore, - MockRfdRevisionStore, MockRfdStore, RfdPdfStore, RfdRevisionMetaStore, - RfdRevisionStore, RfdStore, - }, - NewJob, NewRfd, NewRfdPdf, NewRfdRevision, RfdId, RfdPdfId, RfdRevisionId, - }; + use rfd_model::storage::mock::MockStorage; use rsa::{ pkcs8::{EncodePrivateKey, EncodePublicKey, LineEnding}, RsaPrivateKey, RsaPublicKey, @@ -789,7 +736,7 @@ pub(crate) mod test_mocks { endpoints::login::oauth::{google::GoogleOAuthProvider, OAuthProviderName}, VContext, }; - use v_model::storage::{postgres::PostgresStore, ListPagination, StoreError}; + use v_model::storage::postgres::PostgresStore; use crate::config::{ ContentConfig, GitHubAuthConfig, GitHubConfig, SearchConfig, ServicesConfig, @@ -874,220 +821,4 @@ pub(crate) mod test_mocks { ctx } - - // Construct a mock storage engine that can be wrapped in an ApiContext for testing - pub struct MockStorage { - pub rfd_store: Option>, - pub rfd_revision_store: Option>, - pub rfd_pdf_store: Option>, - pub rfd_revision_meta_store: Option>, - pub job_store: Option>, - } - - impl MockStorage { - pub fn new() -> Self { - Self { - rfd_store: None, - rfd_revision_store: None, - rfd_pdf_store: None, - rfd_revision_meta_store: None, - job_store: None, - } - } - } - - #[async_trait] - impl RfdStore for MockStorage { - async fn get( - &self, - id: &TypedUuid, - deleted: bool, - ) -> Result, StoreError> { - self.rfd_store.as_ref().unwrap().get(id, deleted).await - } - - async fn list( - &self, - filter: rfd_model::storage::RfdFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn upsert(&self, new_rfd: NewRfd) -> Result { - self.rfd_store.as_ref().unwrap().upsert(new_rfd).await - } - - async fn delete( - &self, - id: &TypedUuid, - ) -> Result, StoreError> { - self.rfd_store.as_ref().unwrap().delete(id).await - } - } - - #[async_trait] - impl RfdRevisionStore for MockStorage { - async fn get( - &self, - id: &TypedUuid, - deleted: bool, - ) -> Result, StoreError> { - self.rfd_revision_store - .as_ref() - .unwrap() - .get(id, deleted) - .await - } - - async fn list( - &self, - filter: rfd_model::storage::RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_revision_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn list_unique_rfd( - &self, - filter: rfd_model::storage::RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_revision_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn upsert( - &self, - new_revision: NewRfdRevision, - ) -> Result { - self.rfd_revision_store - .as_ref() - .unwrap() - .upsert(new_revision) - .await - } - - async fn delete( - &self, - id: &TypedUuid, - ) -> Result, StoreError> { - self.rfd_revision_store.as_ref().unwrap().delete(id).await - } - } - - #[async_trait] - impl RfdRevisionMetaStore for MockStorage { - async fn get( - &self, - id: &TypedUuid, - deleted: bool, - ) -> Result, StoreError> { - self.rfd_revision_meta_store - .as_ref() - .unwrap() - .get(id, deleted) - .await - } - - async fn list( - &self, - filter: rfd_model::storage::RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_revision_meta_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn list_unique_rfd( - &self, - filter: rfd_model::storage::RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_revision_meta_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - } - - #[async_trait] - impl RfdPdfStore for MockStorage { - async fn get( - &self, - id: &TypedUuid, - deleted: bool, - ) -> Result, StoreError> { - self.rfd_pdf_store.as_ref().unwrap().get(id, deleted).await - } - - async fn list( - &self, - filter: rfd_model::storage::RfdPdfFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_pdf_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn upsert(&self, new_pdf: NewRfdPdf) -> Result { - self.rfd_pdf_store.as_ref().unwrap().upsert(new_pdf).await - } - - async fn delete( - &self, - id: &TypedUuid, - ) -> Result, StoreError> { - self.rfd_pdf_store.as_ref().unwrap().delete(id).await - } - } - - #[async_trait] - impl JobStore for MockStorage { - async fn get(&self, id: i32) -> Result, StoreError> { - self.job_store.as_ref().unwrap().get(id).await - } - - async fn list( - &self, - filter: rfd_model::storage::JobFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.job_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn upsert(&self, new_job: NewJob) -> Result { - self.job_store.as_ref().unwrap().upsert(new_job).await - } - - async fn start(&self, id: i32) -> Result, StoreError> { - self.job_store.as_ref().unwrap().start(id).await - } - - async fn complete(&self, id: i32) -> Result, StoreError> { - self.job_store.as_ref().unwrap().complete(id).await - } - } } diff --git a/rfd-api/src/endpoints/meta.rs b/rfd-api/src/endpoints/meta.rs index 4bfef56d..f85da40a 100644 --- a/rfd-api/src/endpoints/meta.rs +++ b/rfd-api/src/endpoints/meta.rs @@ -11,7 +11,7 @@ use v_api::ApiContext; use v_model::permissions::Caller; use crate::{ - context::{RfdContext, RfdMeta}, + context::{RfdContext, RfdWithoutContent}, permissions::RfdPermission, util::response::client_error, }; @@ -29,24 +29,24 @@ pub struct RfdPathParams { path = "/meta/rfd/{number}", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn get_rfd_meta( +pub async fn view_rfd_meta( rqctx: RequestContext, path: Path, -) -> Result, HttpError> { +) -> Result, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; - get_rfd_meta_op(ctx, &caller, path.into_inner().number).await + view_rfd_meta_op(ctx, &caller, path.into_inner().number).await } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn get_rfd_meta_op( +async fn view_rfd_meta_op( ctx: &RfdContext, caller: &Caller, number: String, -) -> Result, HttpError> { +) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { Ok(HttpResponseOk( - ctx.get_rfd_meta(caller, rfd_number, None).await?, + ctx.view_rfd_meta(caller, rfd_number, None).await?, )) } else { Err(client_error( @@ -65,19 +65,17 @@ mod tests { use http::StatusCode; use newtype_uuid::{GenericUuid, TypedUuid}; use rfd_model::{ - storage::{MockRfdRevisionMetaStore, MockRfdStore}, - Rfd, RfdRevisionMeta, + schema_ext::ContentFormat, + storage::{mock::MockStorage, MockRfdRevisionMetaStore, MockRfdStore}, + CommitSha, FileSha, Rfd, RfdRevision, RfdRevisionMeta, }; use uuid::Uuid; use v_api::ApiContext; use v_model::{permissions::Caller, Permissions}; use crate::{ - context::{ - test_mocks::{mock_context, MockStorage}, - RfdContext, - }, - endpoints::meta::get_rfd_meta_op, + context::{test_mocks::mock_context, RfdContext}, + endpoints::meta::view_rfd_meta_op, permissions::RfdPermission, }; @@ -93,6 +91,23 @@ mod tests { id: TypedUuid::from_untyped_uuid(private_rfd_id_1), rfd_number: 123, link: None, + content: RfdRevision { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content: String::new(), + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, created_at: Utc::now(), updated_at: Utc::now(), deleted_at: None, @@ -102,6 +117,23 @@ mod tests { id: TypedUuid::from_untyped_uuid(public_rfd_id), rfd_number: 456, link: None, + content: RfdRevision { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content: String::new(), + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, created_at: Utc::now(), updated_at: Utc::now(), deleted_at: None, @@ -111,6 +143,23 @@ mod tests { id: TypedUuid::from_untyped_uuid(private_rfd_id_2), rfd_number: 789, link: None, + content: RfdRevision { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content: String::new(), + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, created_at: Utc::now(), updated_at: Utc::now(), deleted_at: None, @@ -206,12 +255,12 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfdsAll])); - let HttpResponseOk(rfd) = get_rfd_meta_op(&ctx, &caller, "0123".to_string()) + let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0123".to_string()) .await .unwrap(); assert_eq!(123, rfd.rfd_number); - let HttpResponseOk(rfd) = get_rfd_meta_op(&ctx, &caller, "0456".to_string()) + let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0456".to_string()) .await .unwrap(); assert_eq!(456, rfd.rfd_number); @@ -224,12 +273,12 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfd(123)])); - let HttpResponseOk(rfd) = get_rfd_meta_op(&ctx, &caller, "0123".to_string()) + let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0123".to_string()) .await .unwrap(); assert_eq!(123, rfd.rfd_number); - let HttpResponseOk(rfd) = get_rfd_meta_op(&ctx, &caller, "0456".to_string()) + let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0456".to_string()) .await .unwrap(); assert_eq!(456, rfd.rfd_number); @@ -242,7 +291,7 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::::new()); - let result = get_rfd_meta_op(&ctx, &caller, "0123".to_string()).await; + let result = view_rfd_meta_op(&ctx, &caller, "0123".to_string()).await; match result { Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), @@ -260,7 +309,7 @@ mod tests { let ctx = ctx().await; let caller = ctx.v_ctx().builtin_unauthenticated_caller(); - let result = get_rfd_meta_op(&ctx, &caller, "0123".to_string()).await; + let result = view_rfd_meta_op(&ctx, &caller, "0123".to_string()).await; match result { Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), Ok(response) => panic!( diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index fe06ce1a..3563029c 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -23,7 +23,7 @@ use v_model::permissions::Caller; use crate::{ caller::CallerExt, - context::{FullRfd, RfdContext, RfdMeta}, + context::{RfdContext, RfdWithContent, RfdWithoutContent}, permissions::RfdPermission, search::{MeiliSearchResult, SearchRequest}, util::response::{client_error, internal_error, unauthorized}, @@ -36,19 +36,19 @@ use crate::{ path = "/rfd", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn get_rfds( +pub async fn list_rfds( rqctx: RequestContext, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; - get_rfds_op(ctx, &caller).await + list_rfds_op(ctx, &caller).await } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn get_rfds_op( +async fn list_rfds_op( ctx: &RfdContext, caller: &Caller, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let rfds = ctx.list_rfds(caller, None).await?; Ok(HttpResponseOk(rfds)) } @@ -107,23 +107,25 @@ pub struct RfdPathParams { path = "/rfd/{number}", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn get_rfd( +pub async fn view_rfd( rqctx: RequestContext, path: Path, -) -> Result, HttpError> { +) -> Result, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; - get_rfd_op(ctx, &caller, path.into_inner().number).await + view_rfd_op(ctx, &caller, path.into_inner().number).await } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn get_rfd_op( +async fn view_rfd_op( ctx: &RfdContext, caller: &Caller, number: String, -) -> Result, HttpError> { +) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { - Ok(HttpResponseOk(ctx.get_rfd(caller, rfd_number, None).await?)) + Ok(HttpResponseOk( + ctx.view_rfd(caller, rfd_number, None).await?, + )) } else { Err(client_error( ClientErrorStatusCode::BAD_REQUEST, @@ -257,25 +259,25 @@ pub enum RfdAttr { path = "/rfd/{number}/attr/{attr}", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn get_rfd_attr( +pub async fn view_rfd_attr( rqctx: RequestContext, path: Path, ) -> Result, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); - get_rfd_attr_op(ctx, &caller, path.number, path.attr).await + view_rfd_attr_op(ctx, &caller, path.number, path.attr).await } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn get_rfd_attr_op( +async fn view_rfd_attr_op( ctx: &RfdContext, caller: &Caller, number: String, attr: RfdAttrName, ) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { - let rfd = ctx.get_rfd(caller, rfd_number, None).await?; + let rfd = ctx.view_rfd(caller, rfd_number, None).await?; let content = match rfd.format { ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(rfd.content)), ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(rfd.content)), @@ -326,7 +328,7 @@ async fn set_rfd_attr_op( ) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { // Get the latest revision - let revision = ctx.get_rfd_revision(caller, rfd_number, None).await?; + let revision = ctx.view_rfd_revision(caller, rfd_number, None).await?; // TODO: Get rid of these clones let mut content = match revision.content_format { @@ -648,23 +650,24 @@ mod tests { use http::StatusCode; use newtype_uuid::{GenericUuid, TypedUuid}; use rfd_model::{ - storage::{MockRfdPdfStore, MockRfdRevisionMetaStore, MockRfdRevisionStore, MockRfdStore}, - Rfd, RfdRevision, RfdRevisionMeta, + schema_ext::ContentFormat, + storage::{ + mock::MockStorage, MockRfdPdfStore, MockRfdRevisionMetaStore, MockRfdRevisionStore, + MockRfdStore, + }, + CommitSha, FileSha, Rfd, RfdRevision, RfdRevisionMeta, }; use uuid::Uuid; use v_api::ApiContext; use v_model::{permissions::Caller, Permissions}; use crate::{ - context::{ - test_mocks::{mock_context, MockStorage}, - RfdContext, - }, - endpoints::rfd::get_rfd_op, + context::{test_mocks::mock_context, RfdContext}, + endpoints::rfd::view_rfd_op, permissions::RfdPermission, }; - use super::get_rfds_op; + use super::list_rfds_op; async fn ctx() -> RfdContext { let private_rfd_id_1 = Uuid::new_v4(); @@ -678,6 +681,23 @@ mod tests { id: TypedUuid::from_untyped_uuid(private_rfd_id_1), rfd_number: 123, link: None, + content: RfdRevision { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content: String::new(), + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, created_at: Utc::now(), updated_at: Utc::now(), deleted_at: None, @@ -687,6 +707,23 @@ mod tests { id: TypedUuid::from_untyped_uuid(public_rfd_id), rfd_number: 456, link: None, + content: RfdRevision { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content: String::new(), + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, created_at: Utc::now(), updated_at: Utc::now(), deleted_at: None, @@ -696,6 +733,23 @@ mod tests { id: TypedUuid::from_untyped_uuid(private_rfd_id_2), rfd_number: 789, link: None, + content: RfdRevision { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content: String::new(), + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, created_at: Utc::now(), updated_at: Utc::now(), deleted_at: None, @@ -866,7 +920,7 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfdsAll])); - let HttpResponseOk(rfds) = get_rfds_op(&ctx, &caller).await.unwrap(); + let HttpResponseOk(rfds) = list_rfds_op(&ctx, &caller).await.unwrap(); assert_eq!(3, rfds.len()); assert_eq!(789, rfds[0].rfd_number); assert_eq!(456, rfds[1].rfd_number); @@ -874,14 +928,18 @@ mod tests { } #[tokio::test] - async fn get_rfd_via_all_permission() { + async fn view_rfd_via_all_permission() { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfdsAll])); - let HttpResponseOk(rfd) = get_rfd_op(&ctx, &caller, "0123".to_string()).await.unwrap(); + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0123".to_string()) + .await + .unwrap(); assert_eq!(123, rfd.rfd_number); - let HttpResponseOk(rfd) = get_rfd_op(&ctx, &caller, "0456".to_string()).await.unwrap(); + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0456".to_string()) + .await + .unwrap(); assert_eq!(456, rfd.rfd_number); } @@ -892,21 +950,25 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfd(123)])); - let HttpResponseOk(rfds) = get_rfds_op(&ctx, &caller).await.unwrap(); + let HttpResponseOk(rfds) = list_rfds_op(&ctx, &caller).await.unwrap(); assert_eq!(2, rfds.len()); assert_eq!(456, rfds[0].rfd_number); assert_eq!(123, rfds[1].rfd_number); } #[tokio::test] - async fn get_rfd_with_direct_permission() { + async fn view_rfd_with_direct_permission() { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfd(123)])); - let HttpResponseOk(rfd) = get_rfd_op(&ctx, &caller, "0123".to_string()).await.unwrap(); + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0123".to_string()) + .await + .unwrap(); assert_eq!(123, rfd.rfd_number); - let HttpResponseOk(rfd) = get_rfd_op(&ctx, &caller, "0456".to_string()).await.unwrap(); + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0456".to_string()) + .await + .unwrap(); assert_eq!(456, rfd.rfd_number); } @@ -917,17 +979,17 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::::new()); - let HttpResponseOk(rfds) = get_rfds_op(&ctx, &caller).await.unwrap(); + let HttpResponseOk(rfds) = list_rfds_op(&ctx, &caller).await.unwrap(); assert_eq!(1, rfds.len()); assert_eq!(456, rfds[0].rfd_number); } #[tokio::test] - async fn get_rfd_without_permission() { + async fn view_rfd_without_permission() { let ctx = ctx().await; let caller = Caller::from(Permissions::::new()); - let result = get_rfd_op(&ctx, &caller, "0123".to_string()).await; + let result = view_rfd_op(&ctx, &caller, "0123".to_string()).await; match result { Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), @@ -944,19 +1006,20 @@ mod tests { async fn list_rfds_as_unauthenticated() { let ctx = ctx().await; - let HttpResponseOk(rfds) = get_rfds_op(&ctx, &ctx.v_ctx().builtin_unauthenticated_caller()) - .await - .unwrap(); + let HttpResponseOk(rfds) = + list_rfds_op(&ctx, &ctx.v_ctx().builtin_unauthenticated_caller()) + .await + .unwrap(); assert_eq!(1, rfds.len()); assert_eq!(456, rfds[0].rfd_number); } #[tokio::test] - async fn get_rfd_as_unauthenticated() { + async fn view_rfd_as_unauthenticated() { let ctx = ctx().await; let caller = ctx.v_ctx().builtin_unauthenticated_caller(); - let result = get_rfd_op(&ctx, &caller, "0123".to_string()).await; + let result = view_rfd_op(&ctx, &caller, "0123".to_string()).await; match result { Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), Ok(response) => panic!( diff --git a/rfd-api/src/main.rs b/rfd-api/src/main.rs index e11cae4b..41ebc0cb 100644 --- a/rfd-api/src/main.rs +++ b/rfd-api/src/main.rs @@ -3,7 +3,6 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use context::RfdContext; -use rfd_model::storage::postgres::PostgresStore; use server::{server, ServerConfig}; use std::{ net::{SocketAddr, SocketAddrV4}, @@ -18,7 +17,7 @@ use v_api::{ }, ApiContext, VContext, }; -use v_model::storage::postgres::PostgresStore as VApiPostgressStore; +use v_model::storage::postgres::PostgresStore as VApiPostgresStore; use crate::{ config::{AppConfig, ServerLogFormat}, @@ -69,7 +68,7 @@ async fn main() -> anyhow::Result<()> { let mut v_ctx = VContext::new( config.public_url.clone(), Arc::new( - VApiPostgressStore::new(&config.database_url) + VApiPostgresStore::new(&config.database_url) .await .tap_err(|err| { tracing::error!(?err, "Failed to establish initial database connection"); @@ -120,7 +119,7 @@ async fn main() -> anyhow::Result<()> { let context = RfdContext::new( config.public_url, Arc::new( - PostgresStore::new(&config.database_url) + VApiPostgresStore::new(&config.database_url) .await .tap_err(|err| { tracing::error!(?err, "Failed to establish initial database connection"); diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index afacad13..ddb726a3 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -15,10 +15,10 @@ use v_api::{inject_endpoints, v_system_endpoints}; use crate::{ context::RfdContext, endpoints::{ - meta::get_rfd_meta, + meta::view_rfd_meta, 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, list_rfds, publish_rfd, reserve_rfd, search_rfds, set_rfd_attr, + set_rfd_content, set_rfd_document, update_rfd_visibility, view_rfd, view_rfd_attr, }, webhook::github_webhook, }, @@ -74,9 +74,10 @@ pub fn server( inject_endpoints!(api); // RFDs - api.register(get_rfds).expect("Failed to register endpoint"); - api.register(get_rfd).expect("Failed to register endpoint"); - api.register(get_rfd_meta) + api.register(list_rfds) + .expect("Failed to register endpoint"); + api.register(view_rfd).expect("Failed to register endpoint"); + api.register(view_rfd_meta) .expect("Failed to register endpoint"); api.register(reserve_rfd) .expect("Failed to register endpoint"); @@ -84,7 +85,7 @@ pub fn server( .expect("Failed to register endpoint"); api.register(set_rfd_content) .expect("Failed to register endpoint"); - api.register(get_rfd_attr) + api.register(view_rfd_attr) .expect("Failed to register endpoint"); api.register(set_rfd_attr) .expect("Failed to register endpoint"); diff --git a/rfd-cli/src/main.rs b/rfd-cli/src/main.rs index a3ccc6ab..9453ed97 100644 --- a/rfd-cli/src/main.rs +++ b/rfd-cli/src/main.rs @@ -409,7 +409,7 @@ impl ProgenitorCliConfig for Context { .printer() .unwrap() .output_oauth_secret(reserialize(value)), - "Array_of_ListRfd" => self.printer().unwrap().output_rfd_list(reserialize(value)), + "Array_of_RfdMeta" => self.printer().unwrap().output_rfd_list(reserialize(value)), "FullRfd" => self.printer().unwrap().output_rfd_full(reserialize(value)), "Rfd" => self.printer().unwrap().output_rfd(reserialize(value)), "SearchResults" => self diff --git a/rfd-github/src/lib.rs b/rfd-github/src/lib.rs index 542595f4..cc31a773 100644 --- a/rfd-github/src/lib.rs +++ b/rfd-github/src/lib.rs @@ -730,6 +730,28 @@ impl GitHubRfdLocation { } } +#[derive(Clone)] +struct GitHubPullRequestComments { + pub client: Client, +} + +impl GitHubPullRequestComments { + async fn comments(&self) { + let pulls = self.client.pulls(); + let comments = pulls + .list_all_review_comments( + "owner", + "repo", + 0, + octorust::types::Sort::Created, + octorust::types::Order::Desc, + None, + ) + .await + .unwrap(); + } +} + struct FetchedRfdContent { decoded: Vec, parsed: String, diff --git a/rfd-model/src/db.rs b/rfd-model/src/db.rs index 84333931..92aac45c 100644 --- a/rfd-model/src/db.rs +++ b/rfd-model/src/db.rs @@ -3,7 +3,7 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use chrono::{DateTime, Utc}; -use diesel::{Insertable, Queryable}; +use diesel::{Insertable, Queryable, Selectable}; use partial_struct::partial; use serde::{Deserialize, Serialize}; use uuid::Uuid; @@ -13,7 +13,7 @@ use crate::{ schema_ext::{ContentFormat, PdfSource, Visibility}, }; -#[derive(Debug, Deserialize, Serialize, Queryable, Insertable)] +#[derive(Debug, Deserialize, Serialize, Queryable, Insertable, Selectable)] #[diesel(table_name = rfd)] pub struct RfdModel { pub id: Uuid, @@ -26,7 +26,7 @@ pub struct RfdModel { } #[partial(RfdRevisionMetaModel)] -#[derive(Debug, Deserialize, Serialize, Queryable, Insertable)] +#[derive(Debug, Deserialize, Serialize, Queryable, Insertable, Selectable)] #[diesel(table_name = rfd_revision)] pub struct RfdRevisionModel { pub id: Uuid, diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index 72b647d8..0dc53323 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -63,12 +63,16 @@ impl TypedUuidKind for RfdId { } #[partial(NewRfd)] +#[partial(RfdMeta)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct Rfd { pub id: TypedUuid, pub rfd_number: i32, pub link: Option, #[partial(NewRfd(skip))] + #[partial(RfdMeta(retype = RfdRevisionMeta))] + pub content: RfdRevision, + #[partial(NewRfd(skip))] pub created_at: DateTime, #[partial(NewRfd(skip))] pub updated_at: DateTime, @@ -77,16 +81,32 @@ pub struct Rfd { pub visibility: Visibility, } -impl From for Rfd { - fn from(value: RfdModel) -> Self { +impl From<(RfdModel, RfdRevisionModel)> for Rfd { + fn from((rfd, revision): (RfdModel, RfdRevisionModel)) -> Self { Self { - id: TypedUuid::from_untyped_uuid(value.id), - rfd_number: value.rfd_number, - link: value.link, - created_at: value.created_at, - updated_at: value.updated_at, - deleted_at: value.deleted_at, - visibility: value.visibility, + id: TypedUuid::from_untyped_uuid(rfd.id), + rfd_number: rfd.rfd_number, + link: rfd.link, + content: revision.into(), + created_at: rfd.created_at, + updated_at: rfd.updated_at, + deleted_at: rfd.deleted_at, + visibility: rfd.visibility, + } + } +} + +impl From<(RfdModel, RfdRevisionMetaModel)> for RfdMeta { + fn from((rfd, revision): (RfdModel, RfdRevisionMetaModel)) -> Self { + Self { + id: TypedUuid::from_untyped_uuid(rfd.id), + rfd_number: rfd.rfd_number, + link: rfd.link, + content: revision.into(), + created_at: rfd.created_at, + updated_at: rfd.updated_at, + deleted_at: rfd.deleted_at, + visibility: rfd.visibility, } } } diff --git a/rfd-model/src/storage/mock.rs b/rfd-model/src/storage/mock.rs new file mode 100644 index 00000000..0caa5b94 --- /dev/null +++ b/rfd-model/src/storage/mock.rs @@ -0,0 +1,256 @@ +use async_trait::async_trait; +use newtype_uuid::TypedUuid; +use std::sync::Arc; +use v_model::storage::StoreError; + +use crate::{ + Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdMeta, RfdPdf, RfdPdfId, + RfdRevision, RfdRevisionId, RfdRevisionMeta, +}; + +use super::{ + JobFilter, JobStore, ListPagination, MockJobStore, MockRfdMetaStore, MockRfdPdfStore, + MockRfdRevisionMetaStore, MockRfdRevisionStore, MockRfdStore, RfdFilter, RfdMetaStore, + RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, RfdRevisionStore, RfdStore, +}; + +pub struct MockStorage { + pub rfd_store: Option>, + pub rfd_meta_store: Option>, + pub rfd_revision_store: Option>, + pub rfd_revision_meta_store: Option>, + pub rfd_pdf_store: Option>, + pub job_store: Option>, +} + +impl MockStorage { + pub fn new() -> Self { + Self { + rfd_store: None, + rfd_meta_store: None, + rfd_revision_store: None, + rfd_revision_meta_store: None, + rfd_pdf_store: None, + job_store: None, + } + } +} + +#[async_trait] +impl RfdStore for MockStorage { + async fn get( + &self, + id: &TypedUuid, + revision: Option>, + deleted: bool, + ) -> Result, StoreError> { + self.rfd_store + .as_ref() + .unwrap() + .get(id, revision, deleted) + .await + } + + async fn list( + &self, + filter: RfdFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } + + async fn upsert(&self, new_rfd: NewRfd) -> Result { + self.rfd_store.as_ref().unwrap().upsert(new_rfd).await + } + + async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { + self.rfd_store.as_ref().unwrap().delete(id).await + } +} + +#[async_trait] +impl RfdMetaStore for MockStorage { + async fn get( + &self, + id: TypedUuid, + revision: Option>, + deleted: bool, + ) -> Result, StoreError> { + self.rfd_meta_store + .as_ref() + .unwrap() + .get(id, revision, deleted) + .await + } + + async fn list( + &self, + filter: RfdFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_meta_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } +} + +#[async_trait] +impl RfdRevisionStore for MockStorage { + async fn get( + &self, + id: &TypedUuid, + deleted: bool, + ) -> Result, StoreError> { + self.rfd_revision_store + .as_ref() + .unwrap() + .get(id, deleted) + .await + } + + async fn list( + &self, + filter: RfdRevisionFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_revision_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } + + async fn list_unique_rfd( + &self, + filter: RfdRevisionFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_revision_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } + + async fn upsert(&self, new_revision: NewRfdRevision) -> Result { + self.rfd_revision_store + .as_ref() + .unwrap() + .upsert(new_revision) + .await + } + + async fn delete( + &self, + id: &TypedUuid, + ) -> Result, StoreError> { + self.rfd_revision_store.as_ref().unwrap().delete(id).await + } +} + +#[async_trait] +impl RfdRevisionMetaStore for MockStorage { + async fn get( + &self, + id: &TypedUuid, + deleted: bool, + ) -> Result, StoreError> { + self.rfd_revision_meta_store + .as_ref() + .unwrap() + .get(id, deleted) + .await + } + + async fn list( + &self, + filter: RfdRevisionFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_revision_meta_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } + + async fn list_unique_rfd( + &self, + filter: RfdRevisionFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_revision_meta_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } +} + +#[async_trait] +impl RfdPdfStore for MockStorage { + async fn get( + &self, + id: &TypedUuid, + deleted: bool, + ) -> Result, StoreError> { + self.rfd_pdf_store.as_ref().unwrap().get(id, deleted).await + } + + async fn list( + &self, + filter: RfdPdfFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_pdf_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } + + async fn upsert(&self, new_pdf: NewRfdPdf) -> Result { + self.rfd_pdf_store.as_ref().unwrap().upsert(new_pdf).await + } + + async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { + self.rfd_pdf_store.as_ref().unwrap().delete(id).await + } +} + +#[async_trait] +impl JobStore for MockStorage { + async fn get(&self, id: i32) -> Result, StoreError> { + self.job_store.as_ref().unwrap().get(id).await + } + + async fn list( + &self, + filter: JobFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.job_store + .as_ref() + .unwrap() + .list(filter, pagination) + .await + } + + async fn upsert(&self, new_job: NewJob) -> Result { + self.job_store.as_ref().unwrap().upsert(new_job).await + } + + async fn start(&self, id: i32) -> Result, StoreError> { + self.job_store.as_ref().unwrap().start(id).await + } + + async fn complete(&self, id: i32) -> Result, StoreError> { + self.job_store.as_ref().unwrap().complete(id).await + } +} diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index 8eb00d60..94c2b1c4 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -12,16 +12,45 @@ use std::fmt::Debug; use v_model::storage::{ListPagination, StoreError}; use crate::{ - schema_ext::PdfSource, Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdPdf, - RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, + schema_ext::PdfSource, Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdMeta, + RfdPdf, RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, }; +#[cfg(feature = "mock")] +pub mod mock; pub mod postgres; +pub trait RfdStorage: + RfdStore + + RfdMetaStore + + RfdRevisionStore + + RfdRevisionMetaStore + + RfdPdfStore + + JobStore + + Send + + Sync + + 'static +{ +} +impl RfdStorage for T where + T: RfdStore + + RfdMetaStore + + RfdRevisionStore + + RfdRevisionMetaStore + + RfdPdfStore + + JobStore + + Send + + Sync + + 'static +{ +} + #[derive(Debug, Default)] pub struct RfdFilter { pub id: Option>>, + pub revision: Option>>, pub rfd_number: Option>, + pub sha: Option>, pub public: Option, pub deleted: bool, } @@ -32,11 +61,21 @@ impl RfdFilter { self } + pub fn revision(mut self, revision: Option>>) -> Self { + self.revision = revision; + self + } + pub fn rfd_number(mut self, rfd_number: Option>) -> Self { self.rfd_number = rfd_number; self } + pub fn sha(mut self, sha: Option>) -> Self { + self.sha = sha; + self + } + pub fn public(mut self, public: Option) -> Self { self.public = public; self @@ -51,7 +90,12 @@ impl RfdFilter { #[cfg_attr(feature = "mock", automock)] #[async_trait] pub trait RfdStore { - async fn get(&self, id: &TypedUuid, deleted: bool) -> Result, StoreError>; + async fn get( + &self, + id: &TypedUuid, + revision: Option>, + deleted: bool, + ) -> Result, StoreError>; async fn list( &self, filter: RfdFilter, @@ -61,6 +105,22 @@ pub trait RfdStore { async fn delete(&self, id: &TypedUuid) -> Result, StoreError>; } +#[cfg_attr(feature = "mock", automock)] +#[async_trait] +pub trait RfdMetaStore { + async fn get( + &self, + id: TypedUuid, + revision: Option>, + deleted: bool, + ) -> Result, StoreError>; + async fn list( + &self, + filter: RfdFilter, + pagination: &ListPagination, + ) -> Result, StoreError>; +} + // TODO: Make the revision store generic over a revision type. We want to be able to have a metadata // only version of the revision model so that we do not need to always load content from the db diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 6d35b2b0..3b98f791 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -2,74 +2,48 @@ // 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 async_bb8_diesel::{AsyncRunQueryDsl, ConnectionError, ConnectionManager}; +use async_bb8_diesel::AsyncRunQueryDsl; use async_trait::async_trait; -use bb8::Pool; use chrono::Utc; use diesel::{ debug_query, insert_into, - pg::PgConnection, query_dsl::QueryDsl, update, upsert::{excluded, on_constraint}, - ExpressionMethods, + ExpressionMethods, SelectableHelper, }; use newtype_uuid::{GenericUuid, TypedUuid}; -use std::time::Duration; -use thiserror::Error; use uuid::Uuid; +use v_model::storage::postgres::PostgresStore; use crate::{ db::{JobModel, RfdModel, RfdPdfModel, RfdRevisionMetaModel, RfdRevisionModel}, schema::{job, rfd, rfd_pdf, rfd_revision}, schema_ext::Visibility, storage::StoreError, - Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdPdf, RfdPdfId, RfdRevision, - RfdRevisionId, RfdRevisionMeta, + Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdMeta, RfdPdf, RfdPdfId, + RfdRevision, RfdRevisionId, RfdRevisionMeta, }; use super::{ - JobFilter, JobStore, ListPagination, RfdFilter, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, - RfdRevisionMetaStore, RfdRevisionStore, RfdStore, + JobFilter, JobStore, ListPagination, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, + RfdRevisionFilter, RfdRevisionMetaStore, RfdRevisionStore, RfdStore, }; -pub type DbPool = Pool>; - -pub struct PostgresStore { - pool: DbPool, -} - -#[derive(Debug, Error)] -pub enum PostgresError { - #[error("Failed to connect to database")] - Connection(ConnectionError), -} - -impl From for PostgresError { - fn from(error: ConnectionError) -> Self { - PostgresError::Connection(error) - } -} - -impl PostgresStore { - pub async fn new(url: &str) -> Result { - let manager = ConnectionManager::::new(url); - - Ok(Self { - pool: Pool::builder() - .connection_timeout(Duration::from_secs(30)) - .build(manager) - .await?, - }) - } -} - #[async_trait] impl RfdStore for PostgresStore { - async fn get(&self, id: &TypedUuid, deleted: bool) -> Result, StoreError> { + async fn get( + &self, + id: &TypedUuid, + revision: Option>, + deleted: bool, + ) -> Result, StoreError> { let rfd = RfdStore::list( self, - RfdFilter::default().id(Some(vec![*id])).deleted(deleted), + RfdFilter::default() + .id(Some(vec![*id])) + .revision(revision.map(|rev| vec![rev])) + .deleted(deleted), &ListPagination::default().limit(1), ) .await?; @@ -81,13 +55,18 @@ impl RfdStore for PostgresStore { filter: RfdFilter, pagination: &ListPagination, ) -> Result, StoreError> { - let mut query = rfd::dsl::rfd.into_boxed(); + let mut query = rfd::table + .inner_join(rfd_revision::table) + .distinct_on(rfd::id) + .into_boxed(); tracing::trace!(?filter, "Lookup RFDs"); let RfdFilter { id, + revision, rfd_number, + sha, public, deleted, } = filter; @@ -97,10 +76,20 @@ impl RfdStore for PostgresStore { query.filter(rfd::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid))); } + if let Some(revision) = revision { + query = query.filter( + rfd_revision::id.eq_any(revision.into_iter().map(GenericUuid::into_untyped_uuid)), + ); + } + if let Some(rfd_number) = rfd_number { query = query.filter(rfd::rfd_number.eq_any(rfd_number)); } + if let Some(sha) = sha { + query = query.filter(rfd_revision::sha.eq_any(sha)); + } + if let Some(public) = public { query = query.filter( rfd::visibility.eq(public @@ -110,14 +99,19 @@ impl RfdStore for PostgresStore { } if !deleted { - query = query.filter(rfd::deleted_at.is_null()); + query = query + .filter(rfd::deleted_at.is_null()) + .filter(rfd_revision::deleted_at.is_null()); } let results = query .offset(pagination.offset) .limit(pagination.limit) - .order(rfd::rfd_number.desc()) - .get_results_async::(&*self.pool.get().await?) + .order(( + rfd_revision::rfd_id.asc(), + rfd_revision::committed_at.desc(), + )) + .get_results_async::<(RfdModel, RfdRevisionModel)>(&*self.pool.get().await?) .await?; tracing::trace!(count = ?results.len(), "Found RFDs"); @@ -126,7 +120,7 @@ impl RfdStore for PostgresStore { } async fn upsert(&self, new_rfd: NewRfd) -> Result { - let rfd: RfdModel = insert_into(rfd::dsl::rfd) + let _: RfdModel = insert_into(rfd::dsl::rfd) .values(( rfd::id.eq(new_rfd.id.into_untyped_uuid()), rfd::rfd_number.eq(new_rfd.rfd_number.clone()), @@ -144,7 +138,11 @@ impl RfdStore for PostgresStore { .get_result_async(&*self.pool.get().await?) .await?; - Ok(rfd.into()) + // There is a race condition here than case a failure where a delete occurs between + // the upsert and the get + Ok(RfdStore::get(self, &new_rfd.id, None, false) + .await? + .unwrap()) } async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { @@ -154,7 +152,98 @@ impl RfdStore for PostgresStore { .execute_async(&*self.pool.get().await?) .await?; - RfdStore::get(self, id, true).await + RfdStore::get(self, id, None, true).await + } +} + +#[async_trait] +impl RfdMetaStore for PostgresStore { + async fn get( + &self, + id: TypedUuid, + revision: Option>, + deleted: bool, + ) -> Result, StoreError> { + let rfd = RfdMetaStore::list( + self, + RfdFilter::default() + .id(Some(vec![id])) + .revision(revision.map(|rev| vec![rev])) + .deleted(deleted), + &ListPagination::default().limit(1), + ) + .await?; + Ok(rfd.into_iter().nth(0)) + } + + async fn list( + &self, + filter: RfdFilter, + pagination: &ListPagination, + ) -> Result, StoreError> { + let mut query = rfd::table + .inner_join(rfd_revision::table) + .distinct_on(rfd::id) + .select((RfdModel::as_select(), RfdRevisionMetaModel::as_select())) + .into_boxed(); + + tracing::trace!(?filter, "Lookup RFDs"); + + let RfdFilter { + id, + revision, + rfd_number, + sha, + public, + deleted, + } = filter; + + if let Some(id) = id { + query = + query.filter(rfd::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid))); + } + + if let Some(revision) = revision { + query = query.filter( + rfd_revision::id.eq_any(revision.into_iter().map(GenericUuid::into_untyped_uuid)), + ); + } + + if let Some(rfd_number) = rfd_number { + query = query.filter(rfd::rfd_number.eq_any(rfd_number)); + } + + if let Some(sha) = sha { + query = query.filter(rfd_revision::sha.eq_any(sha)); + } + + if let Some(public) = public { + query = query.filter( + rfd::visibility.eq(public + .then(|| Visibility::Public) + .unwrap_or(Visibility::Private)), + ); + } + + if !deleted { + query = query + .filter(rfd::deleted_at.is_null()) + .filter(rfd_revision::deleted_at.is_null()); + } + + let results = query + .offset(pagination.offset) + .limit(pagination.limit) + .order(( + rfd_revision::rfd_id.asc(), + rfd_revision::committed_at.desc(), + )) + .get_results_async::<(RfdModel, RfdRevisionMetaModel)>(&*self.pool.get().await?) + .await?; + + tracing::trace!(count = ?results.len(), "Found RFDs"); + + Ok(results.into_iter().map(|rfd| rfd.into()).collect()) } } diff --git a/rfd-processor/src/context.rs b/rfd-processor/src/context.rs index 1d32fd9d..3e6a6dd0 100644 --- a/rfd-processor/src/context.rs +++ b/rfd-processor/src/context.rs @@ -19,7 +19,7 @@ use octorust::{ }; use reqwest::Error as ReqwestError; use rfd_github::{GitHubError, GitHubRfdRepo}; -use rfd_model::{schema_ext::PdfSource, storage::postgres::PostgresStore}; +use rfd_model::schema_ext::PdfSource; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, RsaPrivateKey, @@ -27,6 +27,7 @@ use rsa::{ use tap::TapFallible; use thiserror::Error; use tracing::instrument; +use v_model::storage::postgres::PostgresStore; use crate::{ pdf::{PdfFileLocation, PdfStorage, RfdPdf, RfdPdfError}, From 01a4c7649d89061bd0a2d830ee0a99c72188b63e Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 09:03:15 -0600 Subject: [PATCH 02/17] Populate meta store --- rfd-api/src/endpoints/meta.rs | 96 +++++++++++++++++++++++++++++++++-- 1 file changed, 93 insertions(+), 3 deletions(-) diff --git a/rfd-api/src/endpoints/meta.rs b/rfd-api/src/endpoints/meta.rs index f85da40a..07b9c330 100644 --- a/rfd-api/src/endpoints/meta.rs +++ b/rfd-api/src/endpoints/meta.rs @@ -65,9 +65,7 @@ mod tests { use http::StatusCode; use newtype_uuid::{GenericUuid, TypedUuid}; use rfd_model::{ - schema_ext::ContentFormat, - storage::{mock::MockStorage, MockRfdRevisionMetaStore, MockRfdStore}, - CommitSha, FileSha, Rfd, RfdRevision, RfdRevisionMeta, + schema_ext::ContentFormat, storage::{mock::MockStorage, MockRfdMetaStore, MockRfdRevisionMetaStore, MockRfdStore}, CommitSha, FileSha, Rfd, RfdMeta, RfdRevision, RfdRevisionMeta }; use uuid::Uuid; use v_api::ApiContext; @@ -179,6 +177,98 @@ mod tests { Ok(results) }); + let mut rfd_meta_store = MockRfdMetaStore::new(); + rfd_meta_store.expect_list().returning(move |filter, _| { + let mut results = vec![ + RfdMeta { + id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + rfd_number: 123, + link: None, + content: RfdRevisionMeta { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + visibility: rfd_model::schema_ext::Visibility::Private, + }, + RfdMeta { + id: TypedUuid::from_untyped_uuid(public_rfd_id), + rfd_number: 456, + link: None, + content: RfdRevisionMeta { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + visibility: rfd_model::schema_ext::Visibility::Public, + }, + RfdMeta { + id: TypedUuid::from_untyped_uuid(private_rfd_id_2), + rfd_number: 789, + link: None, + content: RfdRevisionMeta { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + visibility: rfd_model::schema_ext::Visibility::Private, + }, + ]; + + results.retain(|rfd| { + filter.rfd_number.is_none() + || filter + .rfd_number + .as_ref() + .unwrap() + .contains(&rfd.rfd_number) + }); + + Ok(results) + }); + let mut rfd_revision_meta_store = MockRfdRevisionMetaStore::new(); rfd_revision_meta_store .expect_list() From 48eda0b5a95b7380be3d5dd8cbc6efa4484185ac Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 09:08:11 -0600 Subject: [PATCH 03/17] Add meta to mock store --- rfd-api/src/endpoints/meta.rs | 5 +- rfd-api/src/endpoints/rfd.rs | 99 +++++++++++++++++++++++++++++++++-- 2 files changed, 100 insertions(+), 4 deletions(-) diff --git a/rfd-api/src/endpoints/meta.rs b/rfd-api/src/endpoints/meta.rs index 07b9c330..cd463be5 100644 --- a/rfd-api/src/endpoints/meta.rs +++ b/rfd-api/src/endpoints/meta.rs @@ -65,7 +65,9 @@ mod tests { use http::StatusCode; use newtype_uuid::{GenericUuid, TypedUuid}; use rfd_model::{ - schema_ext::ContentFormat, storage::{mock::MockStorage, MockRfdMetaStore, MockRfdRevisionMetaStore, MockRfdStore}, CommitSha, FileSha, Rfd, RfdMeta, RfdRevision, RfdRevisionMeta + schema_ext::ContentFormat, + storage::{mock::MockStorage, MockRfdMetaStore, MockRfdRevisionMetaStore, MockRfdStore}, + CommitSha, FileSha, Rfd, RfdMeta, RfdRevision, RfdRevisionMeta, }; use uuid::Uuid; use v_api::ApiContext; @@ -333,6 +335,7 @@ mod tests { let mut storage = MockStorage::new(); storage.rfd_store = Some(Arc::new(rfd_store)); + storage.rfd_meta_store = Some(Arc::new(rfd_meta_store)); storage.rfd_revision_meta_store = Some(Arc::new(rfd_revision_meta_store)); mock_context(storage).await diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 3563029c..37aa1b06 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -652,10 +652,10 @@ mod tests { use rfd_model::{ schema_ext::ContentFormat, storage::{ - mock::MockStorage, MockRfdPdfStore, MockRfdRevisionMetaStore, MockRfdRevisionStore, - MockRfdStore, + mock::MockStorage, MockRfdMetaStore, MockRfdPdfStore, MockRfdRevisionMetaStore, + MockRfdRevisionStore, MockRfdStore, }, - CommitSha, FileSha, Rfd, RfdRevision, RfdRevisionMeta, + CommitSha, FileSha, Rfd, RfdMeta, RfdRevision, RfdRevisionMeta, }; use uuid::Uuid; use v_api::ApiContext; @@ -769,6 +769,98 @@ mod tests { Ok(results) }); + let mut rfd_meta_store = MockRfdMetaStore::new(); + rfd_meta_store.expect_list().returning(move |filter, _| { + let mut results = vec![ + RfdMeta { + id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + rfd_number: 123, + link: None, + content: RfdRevisionMeta { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + visibility: rfd_model::schema_ext::Visibility::Private, + }, + RfdMeta { + id: TypedUuid::from_untyped_uuid(public_rfd_id), + rfd_number: 456, + link: None, + content: RfdRevisionMeta { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + visibility: rfd_model::schema_ext::Visibility::Public, + }, + RfdMeta { + id: TypedUuid::from_untyped_uuid(private_rfd_id_2), + rfd_number: 789, + link: None, + content: RfdRevisionMeta { + id: TypedUuid::new_v4(), + rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), + title: String::new(), + state: None, + discussion: None, + authors: None, + labels: None, + content_format: ContentFormat::Asciidoc, + sha: FileSha(String::new()), + commit: CommitSha(String::new()), + committed_at: Utc::now(), + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + }, + created_at: Utc::now(), + updated_at: Utc::now(), + deleted_at: None, + visibility: rfd_model::schema_ext::Visibility::Private, + }, + ]; + + results.retain(|rfd| { + filter.rfd_number.is_none() + || filter + .rfd_number + .as_ref() + .unwrap() + .contains(&rfd.rfd_number) + }); + + Ok(results) + }); + let private_rfd_revision_id_1 = TypedUuid::new_v4(); let public_rfd_revision_id = TypedUuid::new_v4(); let private_rfd_revision_id_2 = TypedUuid::new_v4(); @@ -906,6 +998,7 @@ mod tests { let mut storage = MockStorage::new(); storage.rfd_store = Some(Arc::new(rfd_store)); + storage.rfd_meta_store = Some(Arc::new(rfd_meta_store)); storage.rfd_revision_store = Some(Arc::new(rfd_revision_store)); storage.rfd_revision_meta_store = Some(Arc::new(rfd_revision_meta_store)); storage.rfd_pdf_store = Some(Arc::new(rfd_pdf_store)); From 9fcad9d948d0b77b81e273d5c188b73a4df674db Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 09:09:48 -0600 Subject: [PATCH 04/17] Fix license --- rfd-model/src/storage/mock.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rfd-model/src/storage/mock.rs b/rfd-model/src/storage/mock.rs index 0caa5b94..da5fa830 100644 --- a/rfd-model/src/storage/mock.rs +++ b/rfd-model/src/storage/mock.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 async_trait::async_trait; use newtype_uuid::TypedUuid; use std::sync::Arc; From d77954abb228a48f133242e9cadfdcd0c4f6696c Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 10:39:28 -0600 Subject: [PATCH 05/17] More vec filters --- rfd-api/src/context.rs | 206 ++++----- rfd-api/src/endpoints/meta.rs | 14 +- rfd-api/src/endpoints/rfd.rs | 17 +- rfd-model/src/storage/mock.rs | 48 +-- rfd-model/src/storage/mod.rs | 32 +- rfd-model/src/storage/postgres.rs | 668 ++++++++++++++---------------- rfd-processor/src/processor.rs | 4 +- rfd-processor/src/rfd.rs | 14 +- 8 files changed, 444 insertions(+), 559 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index d1c64ff9..97dc550c 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -17,11 +17,8 @@ use rfd_data::{ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - storage::{ - JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, - RfdRevisionMetaStore, RfdStorage, RfdStore, - }, - CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdRevision, + storage::{JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdStorage, RfdStore}, + CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdRevision, }; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, @@ -29,12 +26,14 @@ use rsa::{ }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::{cmp::Ordering, collections::BTreeSet, sync::Arc}; +use std::{cmp::Ordering, sync::Arc}; use tap::TapFallible; use thiserror::Error; use tracing::instrument; use v_api::{ - response::{resource_not_found, resource_restricted, ResourceResult, ToResourceResult}, + response::{ + resource_not_found, resource_restricted, ResourceError, ResourceResult, ToResourceResult, + }, ApiContext, VContext, }; use v_model::{ @@ -200,81 +199,6 @@ impl RfdContext { // RFD Operations - pub async fn list_rfds( - &self, - caller: &Caller, - filter: Option, - ) -> ResourceResult, StoreError> { - // List all of the RFDs first and then perform filter. This should be be improved once - // filters can be combined to support OR expressions. Effectively we need to be able to - // express "Has access to OR is public" with a filter - let mut rfds = RfdMetaStore::list( - &*self.storage, - filter.unwrap_or_default(), - &ListPagination::default().limit(UNLIMITED), - ) - .await - .tap_err(|err| tracing::error!(?err, "Failed to lookup RFDs")) - .to_resource_result()?; - - // Determine the list of RFDs the caller has direct access to - let direct_access_rfds = caller - .permissions - .iter() - .filter_map(|p| match p { - RfdPermission::GetRfd(number) => Some(*number), - _ => None, - }) - .collect::>(); - - // Filter the list of RFDs down to only those that the caller is allowed to access - rfds.retain_mut(|rfd| { - caller.can(&RfdPermission::GetRfdsAll) - || rfd.visibility == Visibility::Public - || direct_access_rfds.contains(&rfd.rfd_number) - }); - - // Fetch the latest revision for each of the RFDs that is to be returned - let mut rfd_revisions = RfdRevisionMetaStore::list_unique_rfd( - &*self.storage, - RfdRevisionFilter::default().rfd(Some(rfds.iter().map(|rfd| rfd.id).collect())), - &ListPagination::default().limit(UNLIMITED), - ) - .await - .tap_err(|err| tracing::error!(?err, "Failed to lookup RFD revisions")) - .to_resource_result()?; - - // Sort both the RFDs and revisions based on their RFD id to ensure they line up - rfds.sort_by(|a, b| a.id.cmp(&b.id)); - rfd_revisions.sort_by(|a, b| a.rfd_id.cmp(&b.rfd_id)); - - // Zip together the RFDs with their associated revision - let mut rfd_list = rfds - .into_iter() - .zip(rfd_revisions) - .map(|(rfd, revision)| RfdWithoutContent { - id: rfd.id, - rfd_number: rfd.rfd_number, - link: rfd.link, - discussion: revision.discussion, - title: revision.title, - state: revision.state, - authors: revision.authors, - labels: revision.labels, - format: revision.content_format, - sha: revision.sha, - commit: revision.commit.into(), - committed_at: revision.committed_at, - visibility: rfd.visibility, - }) - .collect::>(); - - // Finally sort the RFD list by RFD number - rfd_list.sort_by(|a, b| b.rfd_number.cmp(&a.rfd_number)); - - Ok(rfd_list) - } - #[instrument(skip(self, caller), err(Debug))] pub async fn create_rfd( &self, @@ -352,6 +276,55 @@ impl RfdContext { } } + pub async fn list_rfds( + &self, + caller: &Caller, + filter: Option, + ) -> ResourceResult, StoreError> { + // List all of the RFDs first and then perform filter. This should be be improved once + // filters can be combined to support OR expressions. Effectively we need to be able to + // express "Has access to OR is public" with a filter + let mut rfds = RfdMetaStore::list( + &*self.storage, + filter.map(|filter| vec![filter]).unwrap_or_default(), + &ListPagination::default().limit(UNLIMITED), + ) + .await + .tap_err(|err| tracing::error!(?err, "Failed to lookup RFDs")) + .to_resource_result()?; + + // Filter the list of RFDs down to only those that the caller is allowed to access + rfds.retain_mut(|rfd| { + caller.can(&RfdPermission::GetRfdsAll) + || caller.can(&RfdPermission::GetRfd(rfd.rfd_number)) + || rfd.visibility == Visibility::Public + }); + + let mut rfd_list = rfds + .into_iter() + .map(|rfd| RfdWithoutContent { + id: rfd.id, + rfd_number: rfd.rfd_number, + link: rfd.link, + discussion: rfd.content.discussion, + title: rfd.content.title, + state: rfd.content.state, + authors: rfd.content.authors, + labels: rfd.content.labels, + format: rfd.content.content_format, + sha: rfd.content.sha, + commit: rfd.content.commit.into(), + committed_at: rfd.content.committed_at, + visibility: rfd.visibility, + }) + .collect::>(); + + // Finally sort the RFD list by RFD number + rfd_list.sort_by(|a, b| b.rfd_number.cmp(&a.rfd_number)); + + Ok(rfd_list) + } + #[instrument(skip(self, caller))] async fn get_rfd_by_number( &self, @@ -361,9 +334,9 @@ impl RfdContext { ) -> ResourceResult { let rfd = RfdStore::list( &*self.storage, - RfdFilter::default() + vec![RfdFilter::default() .rfd_number(Some(vec![rfd_number])) - .sha(sha.map(|sha| vec![sha])), + .sha(sha.map(|sha| vec![sha]))], &ListPagination::latest(), ) .await @@ -394,7 +367,7 @@ impl RfdContext { let rfd = self.get_rfd_by_number(caller, rfd_number, sha).await?; let pdfs = RfdPdfStore::list( &*self.storage, - RfdPdfFilter::default().rfd_revision(Some(vec![rfd.content.id])), + vec![RfdPdfFilter::default().rfd_revision(Some(vec![rfd.content.id]))], &ListPagination::default(), ) .await @@ -425,38 +398,6 @@ impl RfdContext { }) } - #[instrument(skip(self, caller))] - async fn get_rfd_meta_by_number( - &self, - caller: &Caller, - rfd_number: i32, - sha: Option, - ) -> ResourceResult { - let rfd = RfdMetaStore::list( - &*self.storage, - RfdFilter::default() - .rfd_number(Some(vec![rfd_number])) - .sha(sha.map(|sha| vec![sha])), - &ListPagination::latest(), - ) - .await - .to_resource_result()? - .pop(); - - if let Some(rfd) = rfd { - if caller.can(&RfdPermission::GetRfdsAll) - || caller.can(&RfdPermission::GetRfd(rfd.rfd_number)) - || rfd.visibility == Visibility::Public - { - Ok(rfd) - } else { - resource_not_found() - } - } else { - resource_not_found() - } - } - #[instrument(skip(self, caller))] pub async fn view_rfd_meta( &self, @@ -464,22 +405,19 @@ impl RfdContext { rfd_number: i32, sha: Option, ) -> ResourceResult { - let rfd = self.get_rfd_meta_by_number(caller, rfd_number, sha).await?; - Ok(RfdWithoutContent { - id: rfd.id, - rfd_number: rfd.rfd_number, - link: rfd.link, - discussion: rfd.content.discussion, - title: rfd.content.title, - state: rfd.content.state, - authors: rfd.content.authors, - labels: rfd.content.labels, - format: rfd.content.content_format, - sha: rfd.content.sha, - commit: rfd.content.commit.into(), - committed_at: rfd.content.committed_at, - visibility: rfd.visibility, - }) + let rfd = self + .list_rfds( + caller, + Some( + RfdFilter::default() + .rfd_number(Some(vec![rfd_number])) + .sha(sha.map(|sha| vec![sha])), + ), + ) + .await? + .pop(); + + rfd.ok_or(ResourceError::DoesNotExist) } #[instrument(skip(self, caller))] @@ -695,7 +633,7 @@ impl RfdContext { ]) { let mut rfds = RfdStore::list( &*self.storage, - RfdFilter::default().rfd_number(Some(vec![rfd_number])), + vec![RfdFilter::default().rfd_number(Some(vec![rfd_number]))], &ListPagination::default().limit(1), ) .await diff --git a/rfd-api/src/endpoints/meta.rs b/rfd-api/src/endpoints/meta.rs index cd463be5..88a0faca 100644 --- a/rfd-api/src/endpoints/meta.rs +++ b/rfd-api/src/endpoints/meta.rs @@ -168,8 +168,9 @@ mod tests { ]; results.retain(|rfd| { - filter.rfd_number.is_none() - || filter + filter.len() == 0 + || filter[0].rfd_number.is_none() + || filter[0] .rfd_number .as_ref() .unwrap() @@ -260,8 +261,9 @@ mod tests { ]; results.retain(|rfd| { - filter.rfd_number.is_none() - || filter + filter.len() == 0 + || filter[0].rfd_number.is_none() + || filter[0] .rfd_number .as_ref() .unwrap() @@ -327,7 +329,9 @@ mod tests { ]; results.retain(|revision| { - filter.rfd.is_none() || filter.rfd.as_ref().unwrap().contains(&revision.rfd_id) + filter.len() == 0 + || filter[0].rfd.is_none() + || filter[0].rfd.as_ref().unwrap().contains(&revision.rfd_id) }); Ok(results) diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 37aa1b06..6c7c0912 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -758,8 +758,9 @@ mod tests { ]; results.retain(|rfd| { - filter.rfd_number.is_none() - || filter + filter.len() == 0 + || filter[0].rfd_number.is_none() + || filter[0] .rfd_number .as_ref() .unwrap() @@ -850,8 +851,9 @@ mod tests { ]; results.retain(|rfd| { - filter.rfd_number.is_none() - || filter + filter.len() == 0 + || filter[0].rfd_number.is_none() + || filter[0] .rfd_number .as_ref() .unwrap() @@ -923,7 +925,8 @@ mod tests { ]; results.retain(|revision| { - filter.rfd.is_none() || filter.rfd.as_ref().unwrap().contains(&revision.rfd_id) + filter[0].rfd.is_none() + || filter[0].rfd.as_ref().unwrap().contains(&revision.rfd_id) }); Ok(results) @@ -985,7 +988,9 @@ mod tests { ]; results.retain(|revision| { - filter.rfd.is_none() || filter.rfd.as_ref().unwrap().contains(&revision.rfd_id) + filter.len() == 0 + || filter[0].rfd.is_none() + || filter[0].rfd.as_ref().unwrap().contains(&revision.rfd_id) }); Ok(results) diff --git a/rfd-model/src/storage/mock.rs b/rfd-model/src/storage/mock.rs index da5fa830..cf3f829c 100644 --- a/rfd-model/src/storage/mock.rs +++ b/rfd-model/src/storage/mock.rs @@ -57,13 +57,13 @@ impl RfdStore for MockStorage { async fn list( &self, - filter: RfdFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { self.rfd_store .as_ref() .unwrap() - .list(filter, pagination) + .list(filters, pagination) .await } @@ -93,13 +93,13 @@ impl RfdMetaStore for MockStorage { async fn list( &self, - filter: RfdFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { self.rfd_meta_store .as_ref() .unwrap() - .list(filter, pagination) + .list(filters, pagination) .await } } @@ -120,25 +120,13 @@ impl RfdRevisionStore for MockStorage { async fn list( &self, - filter: RfdRevisionFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { self.rfd_revision_store .as_ref() .unwrap() - .list(filter, pagination) - .await - } - - async fn list_unique_rfd( - &self, - filter: RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_revision_store - .as_ref() - .unwrap() - .list(filter, pagination) + .list(filters, pagination) .await } @@ -174,25 +162,13 @@ impl RfdRevisionMetaStore for MockStorage { async fn list( &self, - filter: RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - self.rfd_revision_meta_store - .as_ref() - .unwrap() - .list(filter, pagination) - .await - } - - async fn list_unique_rfd( - &self, - filter: RfdRevisionFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { self.rfd_revision_meta_store .as_ref() .unwrap() - .list(filter, pagination) + .list(filters, pagination) .await } } @@ -209,13 +185,13 @@ impl RfdPdfStore for MockStorage { async fn list( &self, - filter: RfdPdfFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { self.rfd_pdf_store .as_ref() .unwrap() - .list(filter, pagination) + .list(filters, pagination) .await } @@ -236,13 +212,13 @@ impl JobStore for MockStorage { async fn list( &self, - filter: JobFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { self.job_store .as_ref() .unwrap() - .list(filter, pagination) + .list(filters, pagination) .await } diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index 94c2b1c4..3546d6bc 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -98,7 +98,7 @@ pub trait RfdStore { ) -> Result, StoreError>; async fn list( &self, - filter: RfdFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; async fn upsert(&self, new_rfd: NewRfd) -> Result; @@ -116,7 +116,7 @@ pub trait RfdMetaStore { ) -> Result, StoreError>; async fn list( &self, - filter: RfdFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; } @@ -171,14 +171,14 @@ pub trait RfdRevisionStore { ) -> Result, StoreError>; async fn list( &self, - filter: RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError>; - async fn list_unique_rfd( - &self, - filter: RfdRevisionFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; + // async fn list_unique_rfd( + // &self, + // filters: Vec, + // pagination: &ListPagination, + // ) -> Result, StoreError>; async fn upsert(&self, new_revision: NewRfdRevision) -> Result; async fn delete( &self, @@ -196,14 +196,14 @@ pub trait RfdRevisionMetaStore { ) -> Result, StoreError>; async fn list( &self, - filter: RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError>; - async fn list_unique_rfd( - &self, - filter: RfdRevisionFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; + // async fn list_unique_rfd( + // &self, + // filter: RfdRevisionFilter, + // pagination: &ListPagination, + // ) -> Result, StoreError>; } #[derive(Debug, Default)] @@ -258,7 +258,7 @@ pub trait RfdPdfStore { ) -> Result, StoreError>; async fn list( &self, - filter: RfdPdfFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; async fn upsert(&self, new_revision: NewRfdPdf) -> Result; @@ -301,7 +301,7 @@ pub trait JobStore { async fn get(&self, id: i32) -> Result, StoreError>; async fn list( &self, - filter: JobFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; async fn upsert(&self, new_job: NewJob) -> Result; diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 3b98f791..d4212de5 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -7,10 +7,12 @@ use async_trait::async_trait; use chrono::Utc; use diesel::{ debug_query, insert_into, + pg::Pg, query_dsl::QueryDsl, + sql_types::Bool, update, upsert::{excluded, on_constraint}, - ExpressionMethods, SelectableHelper, + BoolExpressionMethods, BoxableExpression, ExpressionMethods, SelectableHelper, }; use newtype_uuid::{GenericUuid, TypedUuid}; use uuid::Uuid; @@ -40,10 +42,10 @@ impl RfdStore for PostgresStore { ) -> Result, StoreError> { let rfd = RfdStore::list( self, - RfdFilter::default() + vec![RfdFilter::default() .id(Some(vec![*id])) .revision(revision.map(|rev| vec![rev])) - .deleted(deleted), + .deleted(deleted)], &ListPagination::default().limit(1), ) .await?; @@ -52,7 +54,7 @@ impl RfdStore for PostgresStore { async fn list( &self, - filter: RfdFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { let mut query = rfd::table @@ -60,48 +62,61 @@ impl RfdStore for PostgresStore { .distinct_on(rfd::id) .into_boxed(); - tracing::trace!(?filter, "Lookup RFDs"); + tracing::trace!(?filters, "Lookup RFDs"); - let RfdFilter { - id, - revision, - rfd_number, - sha, - public, - deleted, - } = filter; - - if let Some(id) = id { - query = - query.filter(rfd::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid))); - } - - if let Some(revision) = revision { - query = query.filter( - rfd_revision::id.eq_any(revision.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(rfd_number) = rfd_number { - query = query.filter(rfd::rfd_number.eq_any(rfd_number)); - } - - if let Some(sha) = sha { - query = query.filter(rfd_revision::sha.eq_any(sha)); - } - - if let Some(public) = public { - query = query.filter( - rfd::visibility.eq(public - .then(|| Visibility::Public) - .unwrap_or(Visibility::Private)), - ); - } - - if !deleted { - query = query - .filter(rfd::deleted_at.is_null()) - .filter(rfd_revision::deleted_at.is_null()); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdFilter { + id, + revision, + rfd_number, + sha, + public, + deleted, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(revision) = revision { + predicates + .push(Box::new(rfd_revision::id.eq_any( + revision.into_iter().map(GenericUuid::into_untyped_uuid), + ))); + } + + if let Some(rfd_number) = rfd_number { + predicates.push(Box::new(rfd::rfd_number.eq_any(rfd_number))); + } + + if let Some(sha) = sha { + predicates.push(Box::new(rfd_revision::sha.eq_any(sha))); + } + + if let Some(public) = public { + predicates.push(Box::new( + rfd::visibility.eq(public + .then(|| Visibility::Public) + .unwrap_or(Visibility::Private)), + )); + } + + if !deleted { + predicates.push(Box::new(rfd::deleted_at.is_null())); + predicates.push(Box::new(rfd_revision::deleted_at.is_null())); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); } let results = query @@ -166,10 +181,10 @@ impl RfdMetaStore for PostgresStore { ) -> Result, StoreError> { let rfd = RfdMetaStore::list( self, - RfdFilter::default() + vec![RfdFilter::default() .id(Some(vec![id])) .revision(revision.map(|rev| vec![rev])) - .deleted(deleted), + .deleted(deleted)], &ListPagination::default().limit(1), ) .await?; @@ -178,7 +193,7 @@ impl RfdMetaStore for PostgresStore { async fn list( &self, - filter: RfdFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { let mut query = rfd::table @@ -187,48 +202,61 @@ impl RfdMetaStore for PostgresStore { .select((RfdModel::as_select(), RfdRevisionMetaModel::as_select())) .into_boxed(); - tracing::trace!(?filter, "Lookup RFDs"); - - let RfdFilter { - id, - revision, - rfd_number, - sha, - public, - deleted, - } = filter; - - if let Some(id) = id { - query = - query.filter(rfd::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid))); - } - - if let Some(revision) = revision { - query = query.filter( - rfd_revision::id.eq_any(revision.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } + tracing::trace!(?filters, "Lookup RFDs"); - if let Some(rfd_number) = rfd_number { - query = query.filter(rfd::rfd_number.eq_any(rfd_number)); - } - - if let Some(sha) = sha { - query = query.filter(rfd_revision::sha.eq_any(sha)); - } - - if let Some(public) = public { - query = query.filter( - rfd::visibility.eq(public - .then(|| Visibility::Public) - .unwrap_or(Visibility::Private)), - ); - } - - if !deleted { - query = query - .filter(rfd::deleted_at.is_null()) - .filter(rfd_revision::deleted_at.is_null()); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdFilter { + id, + revision, + rfd_number, + sha, + public, + deleted, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(revision) = revision { + predicates + .push(Box::new(rfd_revision::id.eq_any( + revision.into_iter().map(GenericUuid::into_untyped_uuid), + ))); + } + + if let Some(rfd_number) = rfd_number { + predicates.push(Box::new(rfd::rfd_number.eq_any(rfd_number))); + } + + if let Some(sha) = sha { + predicates.push(Box::new(rfd_revision::sha.eq_any(sha))); + } + + if let Some(public) = public { + predicates.push(Box::new( + rfd::visibility.eq(public + .then(|| Visibility::Public) + .unwrap_or(Visibility::Private)), + )); + } + + if !deleted { + predicates.push(Box::new(rfd::deleted_at.is_null())); + predicates.push(Box::new(rfd_revision::deleted_at.is_null())); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); } let results = query @@ -256,9 +284,9 @@ impl RfdRevisionStore for PostgresStore { ) -> Result, StoreError> { let user = RfdRevisionStore::list( self, - RfdRevisionFilter::default() + vec![RfdRevisionFilter::default() .id(Some(vec![*id])) - .deleted(deleted), + .deleted(deleted)], &ListPagination::default().limit(1), ) .await?; @@ -267,38 +295,51 @@ impl RfdRevisionStore for PostgresStore { async fn list( &self, - filter: RfdRevisionFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { let mut query = rfd_revision::dsl::rfd_revision.into_boxed(); - tracing::trace!(?filter, "Lookup RFD revisions"); - - let RfdRevisionFilter { - id, - rfd, - sha, - deleted, - } = filter; + tracing::trace!(?filters, "Lookup RFD revisions"); - if let Some(id) = id { - query = query.filter( - rfd_revision::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(rfd) = rfd { - query = query.filter( - rfd_revision::rfd_id.eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(sha) = sha { - query = query.filter(rfd_revision::sha.eq_any(sha)); - } - - if !deleted { - query = query.filter(rfd_revision::deleted_at.is_null()); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdRevisionFilter { + id, + rfd, + sha, + deleted, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_revision::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(rfd) = rfd { + predicates.push(Box::new( + rfd_revision::rfd_id + .eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(sha) = sha { + predicates.push(Box::new(rfd_revision::sha.eq_any(sha))); + } + + if !deleted { + predicates.push(Box::new(rfd_revision::deleted_at.is_null())); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); } let query = query @@ -318,67 +359,6 @@ impl RfdRevisionStore for PostgresStore { .collect()) } - // TODO: Refactor into a group by arg in list. Diesel types here are a pain - async fn list_unique_rfd( - &self, - filter: RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - let mut query = rfd_revision::dsl::rfd_revision - .distinct_on(rfd_revision::rfd_id) - .into_boxed(); - - tracing::trace!(rfd_ids = ?filter.rfd.as_ref().map(|list| list.len()), "Lookup unique RFD revisions"); - - let RfdRevisionFilter { - id, - rfd, - sha, - deleted, - } = filter; - - if let Some(id) = id { - query = query.filter( - rfd_revision::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(rfd) = rfd { - query = query.filter( - rfd_revision::rfd_id.eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(sha) = sha { - query = query.filter(rfd_revision::sha.eq_any(sha)); - } - - if !deleted { - query = query.filter(rfd_revision::deleted_at.is_null()); - } - - let query = query - .offset(pagination.offset) - .limit(pagination.limit) - .order(( - rfd_revision::rfd_id.asc(), - rfd_revision::committed_at.desc(), - )); - - tracing::info!(query = ?debug_query(&query), "Run list unique rfds"); - - let results = query - .get_results_async::(&*self.pool.get().await?) - .await?; - - tracing::trace!(count = ?results.len(), "Found unique RFD revisions"); - - Ok(results - .into_iter() - .map(|revision| revision.into()) - .collect()) - } - async fn upsert(&self, new_revision: NewRfdRevision) -> Result { let rfd: RfdRevisionModel = insert_into(rfd_revision::dsl::rfd_revision) .values(( @@ -440,9 +420,9 @@ impl RfdRevisionMetaStore for PostgresStore { ) -> Result, StoreError> { let user = RfdRevisionMetaStore::list( self, - RfdRevisionFilter::default() + vec![RfdRevisionFilter::default() .id(Some(vec![*id])) - .deleted(deleted), + .deleted(deleted)], &ListPagination::default().limit(1), ) .await?; @@ -451,7 +431,7 @@ impl RfdRevisionMetaStore for PostgresStore { async fn list( &self, - filter: RfdRevisionFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { let mut query = rfd_revision::dsl::rfd_revision @@ -473,33 +453,46 @@ impl RfdRevisionMetaStore for PostgresStore { )) .into_boxed(); - tracing::trace!(?filter, "Lookup RFD revision metadata"); - - let RfdRevisionFilter { - id, - rfd, - sha, - deleted, - } = filter; + tracing::trace!(?filters, "Lookup RFD revision metadata"); - if let Some(id) = id { - query = query.filter( - rfd_revision::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(rfd) = rfd { - query = query.filter( - rfd_revision::rfd_id.eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(sha) = sha { - query = query.filter(rfd_revision::sha.eq_any(sha)); - } - - if !deleted { - query = query.filter(rfd_revision::deleted_at.is_null()); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdRevisionFilter { + id, + rfd, + sha, + deleted, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_revision::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(rfd) = rfd { + predicates.push(Box::new( + rfd_revision::rfd_id + .eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(sha) = sha { + predicates.push(Box::new(rfd_revision::sha.eq_any(sha))); + } + + if !deleted { + predicates.push(Box::new(rfd_revision::deleted_at.is_null())); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); } let query = query @@ -520,83 +513,6 @@ impl RfdRevisionMetaStore for PostgresStore { .map(|revision| revision.into()) .collect()) } - - // TODO: Refactor into a group by arg in list. Diesel types here are a pain - async fn list_unique_rfd( - &self, - filter: RfdRevisionFilter, - pagination: &ListPagination, - ) -> Result, StoreError> { - let mut query = rfd_revision::dsl::rfd_revision - .select(( - rfd_revision::id, - rfd_revision::rfd_id, - rfd_revision::title, - rfd_revision::state, - rfd_revision::discussion, - rfd_revision::authors, - rfd_revision::content_format, - rfd_revision::sha, - rfd_revision::commit_sha, - rfd_revision::committed_at, - rfd_revision::created_at, - rfd_revision::updated_at, - rfd_revision::deleted_at, - rfd_revision::labels, - )) - .distinct_on(rfd_revision::rfd_id) - .into_boxed(); - - tracing::trace!(rfd_ids = ?filter.rfd.as_ref().map(|list| list.len()), "Lookup unique RFD revision metadata"); - - let RfdRevisionFilter { - id, - rfd, - sha, - deleted, - } = filter; - - if let Some(id) = id { - query = query.filter( - rfd_revision::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(rfd) = rfd { - query = query.filter( - rfd_revision::rfd_id.eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(sha) = sha { - query = query.filter(rfd_revision::sha.eq_any(sha)); - } - - if !deleted { - query = query.filter(rfd_revision::deleted_at.is_null()); - } - - let query = query - .offset(pagination.offset) - .limit(pagination.limit) - .order(( - rfd_revision::rfd_id.asc(), - rfd_revision::committed_at.desc(), - )); - - tracing::info!(query = ?debug_query(&query), "Run list unique rfd metadata"); - - let results = query - .get_results_async::(&*self.pool.get().await?) - .await?; - - tracing::trace!(count = ?results.len(), "Found unique RFD revision metadata"); - - Ok(results - .into_iter() - .map(|revision| revision.into()) - .collect()) - } } #[async_trait] @@ -608,7 +524,7 @@ impl RfdPdfStore for PostgresStore { ) -> Result, StoreError> { let user = RfdPdfStore::list( self, - RfdPdfFilter::default().id(Some(vec![*id])).deleted(deleted), + vec![RfdPdfFilter::default().id(Some(vec![*id])).deleted(deleted)], &ListPagination::default().limit(1), ) .await?; @@ -617,50 +533,63 @@ impl RfdPdfStore for PostgresStore { async fn list( &self, - filter: super::RfdPdfFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { let mut query = rfd_pdf::dsl::rfd_pdf.into_boxed(); - tracing::trace!(?filter, "Lookup RFD pdfs"); - - let RfdPdfFilter { - id, - rfd_revision, - source, - deleted, - rfd, - external_id, - } = filter; - - if let Some(id) = id { - query = query - .filter(rfd_pdf::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid))); - } - - if let Some(rfd_revision) = rfd_revision { - query = query.filter( - rfd_pdf::rfd_revision_id - .eq_any(rfd_revision.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(source) = source { - query = query.filter(rfd_pdf::source.eq_any(source)); - } - - if let Some(rfd) = rfd { - query = query.filter( - rfd_pdf::rfd_id.eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), - ); - } - - if let Some(external_id) = external_id { - query = query.filter(rfd_pdf::external_id.eq_any(external_id)); - } + tracing::trace!(?filters, "Lookup RFD pdfs"); - if !deleted { - query = query.filter(rfd_pdf::deleted_at.is_null()); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdPdfFilter { + id, + rfd_revision, + source, + deleted, + rfd, + external_id, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_pdf::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(rfd_revision) = rfd_revision { + predicates + .push(Box::new(rfd_pdf::rfd_revision_id.eq_any( + rfd_revision.into_iter().map(GenericUuid::into_untyped_uuid), + ))); + } + + if let Some(source) = source { + predicates.push(Box::new(rfd_pdf::source.eq_any(source))); + } + + if let Some(rfd) = rfd { + predicates.push(Box::new( + rfd_pdf::rfd_id.eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(external_id) = external_id { + predicates.push(Box::new(rfd_pdf::external_id.eq_any(external_id))); + } + + if !deleted { + predicates.push(Box::new(rfd_pdf::deleted_at.is_null())); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); } let results = query @@ -710,7 +639,7 @@ impl JobStore for PostgresStore { async fn get(&self, id: i32) -> Result, StoreError> { let user = JobStore::list( self, - JobFilter::default().id(Some(vec![id])), + vec![JobFilter::default().id(Some(vec![id]))], &ListPagination::default().limit(1), ) .await?; @@ -719,36 +648,47 @@ impl JobStore for PostgresStore { async fn list( &self, - filter: super::JobFilter, + filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { let mut query = job::dsl::job.into_boxed(); - - let JobFilter { - id, - sha, - processed, - started, - } = filter; - - if let Some(id) = id { - query = query.filter(job::id.eq_any(id)); - } - - if let Some(sha) = sha { - query = query.filter(job::sha.eq_any(sha)); - } - - if let Some(processed) = processed { - query = query.filter(job::processed.eq(processed)); - } - - if let Some(started) = started { - if started { - query = query.filter(job::started_at.is_not_null()); - } else { - query = query.filter(job::started_at.is_null()); - } + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let JobFilter { + id, + sha, + processed, + started, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new(job::id.eq_any(id))); + } + + if let Some(sha) = sha { + predicates.push(Box::new(job::sha.eq_any(sha))); + } + + if let Some(processed) = processed { + predicates.push(Box::new(job::processed.eq(processed))); + } + + if let Some(started) = started { + if started { + predicates.push(Box::new(job::started_at.is_not_null())); + } else { + predicates.push(Box::new(job::started_at.is_null())); + } + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); } let results = query @@ -803,3 +743,25 @@ impl JobStore for PostgresStore { JobStore::get(self, id).await } } + +fn flatten_predicates( + predicates: Vec>>>, +) -> Option>> +where + T: 'static, +{ + let mut filter_predicates = vec![]; + + for p in predicates { + let flat = p + .into_iter() + .reduce(|combined, entry| Box::new(combined.and(entry))); + if let Some(flat) = flat { + filter_predicates.push(flat); + } + } + + filter_predicates + .into_iter() + .reduce(|combined, entry| Box::new(combined.or(entry))) +} diff --git a/rfd-processor/src/processor.rs b/rfd-processor/src/processor.rs index d73814dc..b0d8174d 100644 --- a/rfd-processor/src/processor.rs +++ b/rfd-processor/src/processor.rs @@ -34,9 +34,9 @@ pub async fn processor(ctx: Arc) -> Result<(), JobError> { if ctx.processor.enabled { let jobs = JobStore::list( &ctx.db.storage, - JobFilter::default() + vec![JobFilter::default() .processed(Some(false)) - .started(Some(false)), + .started(Some(false))], &pagination, ) .await?; diff --git a/rfd-processor/src/rfd.rs b/rfd-processor/src/rfd.rs index 5cb4bf5c..70d2bfbe 100644 --- a/rfd-processor/src/rfd.rs +++ b/rfd-processor/src/rfd.rs @@ -56,7 +56,7 @@ impl PersistedRfd { { let existing_rfd = RfdStore::list( storage, - RfdFilter::default().rfd_number(Some(vec![number.into()])), + vec![RfdFilter::default().rfd_number(Some(vec![number.into()]))], &ListPagination::latest(), ) .await? @@ -66,7 +66,7 @@ impl PersistedRfd { if let Some(rfd) = existing_rfd { let most_recent_revision = RfdRevisionStore::list( storage, - RfdRevisionFilter::default().rfd(Some(vec![rfd.id])), + vec![RfdRevisionFilter::default().rfd(Some(vec![rfd.id]))], &ListPagination::latest(), ) .await? @@ -75,7 +75,7 @@ impl PersistedRfd { let most_recent_pdf = RfdPdfStore::list( storage, - RfdPdfFilter::default().rfd(Some(vec![rfd.id])), + vec![RfdPdfFilter::default().rfd(Some(vec![rfd.id]))], &ListPagination::latest(), ) .await? @@ -323,7 +323,7 @@ impl RemoteRfd { let (id, visibility) = RfdStore::list( storage, - RfdFilter::default().rfd_number(Some(vec![payload.number.into()])), + vec![RfdFilter::default().rfd_number(Some(vec![payload.number.into()]))], &ListPagination::latest(), ) .await? @@ -345,9 +345,9 @@ impl RemoteRfd { let id = RfdRevisionStore::list( storage, - RfdRevisionFilter::default() + vec![RfdRevisionFilter::default() .rfd(Some(vec![rfd.id])) - .sha(Some(vec![payload.commit_sha.clone().into()])), + .sha(Some(vec![payload.commit_sha.clone().into()]))], &ListPagination::latest(), ) .await? @@ -395,7 +395,7 @@ impl RemoteRfd { let mut existing_pdf = RfdPdfStore::list( storage, - RfdPdfFilter::default().rfd(Some(vec![rfd.id])), + vec![RfdPdfFilter::default().rfd(Some(vec![rfd.id]))], &ListPagination::latest(), ) .await?; From ef9f0ef6a0cc3c3b81bb059c8580b845ed7a1a98 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 13:27:59 -0600 Subject: [PATCH 06/17] Api surface update --- rfd-api/src/context.rs | 209 +++++---- rfd-api/src/endpoints/meta.rs | 418 ------------------ rfd-api/src/endpoints/mod.rs | 1 - rfd-api/src/endpoints/rfd.rs | 710 +++++++++++++++++++----------- rfd-api/src/server.rs | 32 +- rfd-model/src/lib.rs | 11 + rfd-model/src/storage/mod.rs | 10 +- rfd-model/src/storage/postgres.rs | 16 +- 8 files changed, 637 insertions(+), 770 deletions(-) delete mode 100644 rfd-api/src/endpoints/meta.rs diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 97dc550c..a582ddf1 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -17,8 +17,8 @@ use rfd_data::{ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - storage::{JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdStorage, RfdStore}, - CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdRevision, + storage::{JobStore, RfdFilter, RfdMetaStore, RfdStorage, RfdStore}, + CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdRevision, RfdRevisionId, }; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, @@ -31,9 +31,7 @@ use tap::TapFallible; use thiserror::Error; use tracing::instrument; use v_api::{ - response::{ - resource_not_found, resource_restricted, ResourceError, ResourceResult, ToResourceResult, - }, + response::{resource_not_found, resource_restricted, ResourceResult, ToResourceResult}, ApiContext, VContext, }; use v_model::{ @@ -107,17 +105,74 @@ pub struct RfdWithContent { pub sha: FileSha, pub commit: CommitSha, pub committed_at: DateTime, - #[partial(RfdWithoutContent(skip))] - pub pdfs: Vec, pub visibility: Visibility, } +impl From for RfdWithContent { + fn from(value: Rfd) -> Self { + RfdWithContent { + id: value.id, + rfd_number: value.rfd_number, + link: value.link, + discussion: value.content.discussion, + title: value.content.title, + state: value.content.state, + authors: value.content.authors, + labels: value.content.labels, + content: value.content.content, + format: value.content.content_format, + sha: value.content.sha, + commit: value.content.commit.into(), + committed_at: value.content.committed_at, + visibility: value.visibility, + } + } +} + +impl From for RfdWithoutContent { + fn from(value: RfdMeta) -> Self { + RfdWithoutContent { + id: value.id, + rfd_number: value.rfd_number, + link: value.link, + discussion: value.content.discussion, + title: value.content.title, + state: value.content.state, + authors: value.content.authors, + labels: value.content.labels, + format: value.content.content_format, + sha: value.content.sha, + commit: value.content.commit.into(), + committed_at: value.content.committed_at, + visibility: value.visibility, + } + } +} + #[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] pub struct PdfEntry { pub source: String, pub link: String, } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +pub enum RfdRevisionIdentifier { + Commit(CommitSha), + Id(TypedUuid), +} + +impl From for RfdRevisionIdentifier { + fn from(value: CommitSha) -> Self { + Self::Commit(value) + } +} + +impl From> for RfdRevisionIdentifier { + fn from(value: TypedUuid) -> Self { + Self::Id(value) + } +} + impl RfdContext { pub async fn new( public_url: String, @@ -326,22 +381,60 @@ impl RfdContext { } #[instrument(skip(self, caller))] - async fn get_rfd_by_number( + async fn get_rfd( &self, caller: &Caller, rfd_number: i32, - sha: Option, + revision: Option, ) -> ResourceResult { - let rfd = RfdStore::list( - &*self.storage, - vec![RfdFilter::default() - .rfd_number(Some(vec![rfd_number])) - .sha(sha.map(|sha| vec![sha]))], - &ListPagination::latest(), - ) - .await - .to_resource_result()? - .pop(); + let mut filter = RfdFilter::default().rfd_number(Some(vec![rfd_number])); + filter = match revision { + Some(RfdRevisionIdentifier::Commit(commit_sha)) => { + filter.commit_sha(Some(vec![commit_sha])) + } + Some(RfdRevisionIdentifier::Id(revision)) => filter.revision(Some(vec![revision])), + None => filter, + }; + + let rfd = RfdStore::list(&*self.storage, vec![filter], &ListPagination::latest()) + .await + .to_resource_result()? + .pop(); + + if let Some(rfd) = rfd { + if caller.can(&RfdPermission::GetRfdsAll) + || caller.can(&RfdPermission::GetRfd(rfd.rfd_number)) + || rfd.visibility == Visibility::Public + { + Ok(rfd) + } else { + resource_not_found() + } + } else { + resource_not_found() + } + } + + #[instrument(skip(self, caller))] + async fn get_rfd_meta( + &self, + caller: &Caller, + rfd_number: i32, + revision: Option, + ) -> ResourceResult { + let mut filter = RfdFilter::default().rfd_number(Some(vec![rfd_number])); + filter = match revision { + Some(RfdRevisionIdentifier::Commit(commit_sha)) => { + filter.commit_sha(Some(vec![commit_sha])) + } + Some(RfdRevisionIdentifier::Id(revision)) => filter.revision(Some(vec![revision])), + None => filter, + }; + + let rfd = RfdMetaStore::list(&*self.storage, vec![filter], &ListPagination::latest()) + .await + .to_resource_result()? + .pop(); if let Some(rfd) = rfd { if caller.can(&RfdPermission::GetRfdsAll) @@ -362,40 +455,10 @@ impl RfdContext { &self, caller: &Caller, rfd_number: i32, - sha: Option, + revision: Option, ) -> ResourceResult { - let rfd = self.get_rfd_by_number(caller, rfd_number, sha).await?; - let pdfs = RfdPdfStore::list( - &*self.storage, - vec![RfdPdfFilter::default().rfd_revision(Some(vec![rfd.content.id]))], - &ListPagination::default(), - ) - .await - .to_resource_result()?; - - Ok(RfdWithContent { - id: rfd.id, - rfd_number: rfd.rfd_number, - link: rfd.link, - discussion: rfd.content.discussion, - title: rfd.content.title, - state: rfd.content.state, - authors: rfd.content.authors, - labels: rfd.content.labels, - content: rfd.content.content, - format: rfd.content.content_format, - sha: rfd.content.sha, - commit: rfd.content.commit.into(), - committed_at: rfd.content.committed_at, - pdfs: pdfs - .into_iter() - .map(|pdf| PdfEntry { - source: pdf.source.to_string(), - link: pdf.link, - }) - .collect(), - visibility: rfd.visibility, - }) + let rfd = self.get_rfd(caller, rfd_number, revision).await?; + Ok(rfd.into()) } #[instrument(skip(self, caller))] @@ -403,21 +466,10 @@ impl RfdContext { &self, caller: &Caller, rfd_number: i32, - sha: Option, + revision: Option, ) -> ResourceResult { - let rfd = self - .list_rfds( - caller, - Some( - RfdFilter::default() - .rfd_number(Some(vec![rfd_number])) - .sha(sha.map(|sha| vec![sha])), - ), - ) - .await? - .pop(); - - rfd.ok_or(ResourceError::DoesNotExist) + let rfd = self.get_rfd_meta(caller, rfd_number, revision).await?; + Ok(rfd.into()) } #[instrument(skip(self, caller))] @@ -425,9 +477,9 @@ impl RfdContext { &self, caller: &Caller, rfd_number: i32, - sha: Option, + revision: Option, ) -> ResourceResult { - let rfd = self.get_rfd_by_number(caller, rfd_number, sha).await?; + let rfd = self.get_rfd(caller, rfd_number, revision).await?; Ok(rfd.content) } @@ -631,22 +683,11 @@ impl RfdContext { &RfdPermission::ManageRfdVisibility(rfd_number), &RfdPermission::ManageRfdsVisibilityAll, ]) { - let mut rfds = RfdStore::list( - &*self.storage, - vec![RfdFilter::default().rfd_number(Some(vec![rfd_number]))], - &ListPagination::default().limit(1), - ) - .await - .to_resource_result()?; - - if let Some(mut rfd) = rfds.pop() { - rfd.visibility = visibility; - RfdStore::upsert(&*self.storage, rfd.into()) - .await - .to_resource_result() - } else { - resource_not_found() - } + let mut rfd = self.get_rfd_meta(caller, rfd_number, None).await?; + rfd.visibility = visibility; + RfdStore::upsert(&*self.storage, rfd.into()) + .await + .to_resource_result() } else { resource_restricted() } diff --git a/rfd-api/src/endpoints/meta.rs b/rfd-api/src/endpoints/meta.rs deleted file mode 100644 index 88a0faca..00000000 --- a/rfd-api/src/endpoints/meta.rs +++ /dev/null @@ -1,418 +0,0 @@ -// 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, ClientErrorStatusCode, HttpError, HttpResponseOk, Path, RequestContext}; -use schemars::JsonSchema; -use serde::Deserialize; -use trace_request::trace_request; -use tracing::instrument; -use v_api::ApiContext; -use v_model::permissions::Caller; - -use crate::{ - context::{RfdContext, RfdWithoutContent}, - permissions::RfdPermission, - util::response::client_error, -}; - -#[derive(Debug, Deserialize, JsonSchema)] -pub struct RfdPathParams { - /// The RFD number (examples: 1 or 123) - number: String, -} - -/// Get the latest representation of a RFD -#[trace_request] -#[endpoint { - method = GET, - path = "/meta/rfd/{number}", -}] -#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn view_rfd_meta( - rqctx: RequestContext, - path: Path, -) -> Result, HttpError> { - let ctx = rqctx.context(); - let caller = ctx.v_ctx().get_caller(&rqctx).await?; - view_rfd_meta_op(ctx, &caller, path.into_inner().number).await -} - -#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn view_rfd_meta_op( - ctx: &RfdContext, - caller: &Caller, - number: String, -) -> Result, HttpError> { - if let Ok(rfd_number) = number.parse::() { - Ok(HttpResponseOk( - ctx.view_rfd_meta(caller, rfd_number, None).await?, - )) - } else { - Err(client_error( - ClientErrorStatusCode::BAD_REQUEST, - "Malformed RFD number", - )) - } -} - -#[cfg(test)] -mod tests { - use std::sync::Arc; - - use chrono::Utc; - use dropshot::HttpResponseOk; - use http::StatusCode; - use newtype_uuid::{GenericUuid, TypedUuid}; - use rfd_model::{ - schema_ext::ContentFormat, - storage::{mock::MockStorage, MockRfdMetaStore, MockRfdRevisionMetaStore, MockRfdStore}, - CommitSha, FileSha, Rfd, RfdMeta, RfdRevision, RfdRevisionMeta, - }; - use uuid::Uuid; - use v_api::ApiContext; - use v_model::{permissions::Caller, Permissions}; - - use crate::{ - context::{test_mocks::mock_context, RfdContext}, - endpoints::meta::view_rfd_meta_op, - permissions::RfdPermission, - }; - - async fn ctx() -> RfdContext { - let private_rfd_id_1 = Uuid::new_v4(); - let private_rfd_id_2 = Uuid::new_v4(); - let public_rfd_id = Uuid::new_v4(); - - let mut rfd_store = MockRfdStore::new(); - rfd_store.expect_list().returning(move |filter, _| { - let mut results = vec![ - Rfd { - id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - rfd_number: 123, - link: None, - content: RfdRevision { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: String::new(), - state: None, - discussion: None, - authors: None, - labels: None, - content: String::new(), - content_format: ContentFormat::Asciidoc, - sha: FileSha(String::new()), - commit: CommitSha(String::new()), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - visibility: rfd_model::schema_ext::Visibility::Private, - }, - Rfd { - id: TypedUuid::from_untyped_uuid(public_rfd_id), - rfd_number: 456, - link: None, - content: RfdRevision { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: String::new(), - state: None, - discussion: None, - authors: None, - labels: None, - content: String::new(), - content_format: ContentFormat::Asciidoc, - sha: FileSha(String::new()), - commit: CommitSha(String::new()), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - visibility: rfd_model::schema_ext::Visibility::Public, - }, - Rfd { - id: TypedUuid::from_untyped_uuid(private_rfd_id_2), - rfd_number: 789, - link: None, - content: RfdRevision { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: String::new(), - state: None, - discussion: None, - authors: None, - labels: None, - content: String::new(), - content_format: ContentFormat::Asciidoc, - sha: FileSha(String::new()), - commit: CommitSha(String::new()), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - visibility: rfd_model::schema_ext::Visibility::Private, - }, - ]; - - results.retain(|rfd| { - filter.len() == 0 - || filter[0].rfd_number.is_none() - || filter[0] - .rfd_number - .as_ref() - .unwrap() - .contains(&rfd.rfd_number) - }); - - Ok(results) - }); - - let mut rfd_meta_store = MockRfdMetaStore::new(); - rfd_meta_store.expect_list().returning(move |filter, _| { - let mut results = vec![ - RfdMeta { - id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - rfd_number: 123, - link: None, - content: RfdRevisionMeta { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: String::new(), - state: None, - discussion: None, - authors: None, - labels: None, - content_format: ContentFormat::Asciidoc, - sha: FileSha(String::new()), - commit: CommitSha(String::new()), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - visibility: rfd_model::schema_ext::Visibility::Private, - }, - RfdMeta { - id: TypedUuid::from_untyped_uuid(public_rfd_id), - rfd_number: 456, - link: None, - content: RfdRevisionMeta { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: String::new(), - state: None, - discussion: None, - authors: None, - labels: None, - content_format: ContentFormat::Asciidoc, - sha: FileSha(String::new()), - commit: CommitSha(String::new()), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - visibility: rfd_model::schema_ext::Visibility::Public, - }, - RfdMeta { - id: TypedUuid::from_untyped_uuid(private_rfd_id_2), - rfd_number: 789, - link: None, - content: RfdRevisionMeta { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: String::new(), - state: None, - discussion: None, - authors: None, - labels: None, - content_format: ContentFormat::Asciidoc, - sha: FileSha(String::new()), - commit: CommitSha(String::new()), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - visibility: rfd_model::schema_ext::Visibility::Private, - }, - ]; - - results.retain(|rfd| { - filter.len() == 0 - || filter[0].rfd_number.is_none() - || filter[0] - .rfd_number - .as_ref() - .unwrap() - .contains(&rfd.rfd_number) - }); - - Ok(results) - }); - - let mut rfd_revision_meta_store = MockRfdRevisionMetaStore::new(); - rfd_revision_meta_store - .expect_list() - .returning(move |filter, _| { - let mut results = vec![ - RfdRevisionMeta { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_1), - title: "Private Test RFD 1".to_string(), - state: None, - discussion: None, - authors: None, - labels: None, - content_format: rfd_model::schema_ext::ContentFormat::Asciidoc, - sha: String::new().into(), - commit: String::new().into(), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - RfdRevisionMeta { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(public_rfd_id), - title: "Public Test RFD".to_string(), - state: None, - discussion: None, - authors: None, - labels: None, - content_format: rfd_model::schema_ext::ContentFormat::Asciidoc, - sha: String::new().into(), - commit: String::new().into(), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - RfdRevisionMeta { - id: TypedUuid::new_v4(), - rfd_id: TypedUuid::from_untyped_uuid(private_rfd_id_2), - title: "Private Test RFD 2".to_string(), - state: None, - discussion: None, - authors: None, - labels: None, - content_format: rfd_model::schema_ext::ContentFormat::Asciidoc, - sha: String::new().into(), - commit: String::new().into(), - committed_at: Utc::now(), - created_at: Utc::now(), - updated_at: Utc::now(), - deleted_at: None, - }, - ]; - - results.retain(|revision| { - filter.len() == 0 - || filter[0].rfd.is_none() - || filter[0].rfd.as_ref().unwrap().contains(&revision.rfd_id) - }); - - Ok(results) - }); - - let mut storage = MockStorage::new(); - storage.rfd_store = Some(Arc::new(rfd_store)); - storage.rfd_meta_store = Some(Arc::new(rfd_meta_store)); - storage.rfd_revision_meta_store = Some(Arc::new(rfd_revision_meta_store)); - - mock_context(storage).await - } - - // Test RFD access via the global All RFDs permission - - #[tokio::test] - async fn get_rfd_via_all_permission() { - let ctx = ctx().await; - let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfdsAll])); - - let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0123".to_string()) - .await - .unwrap(); - assert_eq!(123, rfd.rfd_number); - - let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0456".to_string()) - .await - .unwrap(); - assert_eq!(456, rfd.rfd_number); - } - - // Test RFD access via the direct permission to a RFD - - #[tokio::test] - async fn get_rfd_with_direct_permission() { - let ctx = ctx().await; - let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfd(123)])); - - let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0123".to_string()) - .await - .unwrap(); - assert_eq!(123, rfd.rfd_number); - - let HttpResponseOk(rfd) = view_rfd_meta_op(&ctx, &caller, "0456".to_string()) - .await - .unwrap(); - assert_eq!(456, rfd.rfd_number); - } - - // Test RFD access fails when a caller does not have permission - - #[tokio::test] - async fn get_rfd_without_permission() { - let ctx = ctx().await; - let caller = Caller::from(Permissions::::new()); - - let result = view_rfd_meta_op(&ctx, &caller, "0123".to_string()).await; - - match result { - Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), - Ok(response) => panic!( - "Expected a 404 error, but instead found a RFD {:?}", - response.0 - ), - } - } - - // Test RFD access to public RFDs as the unauthenticated user - - #[tokio::test] - async fn get_rfd_as_unauthenticated() { - let ctx = ctx().await; - let caller = ctx.v_ctx().builtin_unauthenticated_caller(); - - let result = view_rfd_meta_op(&ctx, &caller, "0123".to_string()).await; - match result { - Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), - Ok(response) => panic!( - "Expected a 404 error, but instead found a RFD {:?}", - response.0 - ), - } - } -} diff --git a/rfd-api/src/endpoints/mod.rs b/rfd-api/src/endpoints/mod.rs index 7107ea73..edacafe6 100644 --- a/rfd-api/src/endpoints/mod.rs +++ b/rfd-api/src/endpoints/mod.rs @@ -2,6 +2,5 @@ // 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/. -pub mod meta; pub mod rfd; pub mod webhook; diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 6c7c0912..372ffa18 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -6,13 +6,14 @@ use dropshot::{ endpoint, ClientErrorStatusCode, HttpError, HttpResponseAccepted, HttpResponseOk, Path, Query, RequestContext, TypedBody, }; +use newtype_uuid::TypedUuid; use rfd_data::{ content::{RfdAsciidoc, RfdContent, RfdDocument, RfdMarkdown}, RfdState, }; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - Rfd, + Rfd, RfdRevisionId, }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -23,12 +24,62 @@ use v_model::permissions::Caller; use crate::{ caller::CallerExt, - context::{RfdContext, RfdWithContent, RfdWithoutContent}, + context::{RfdContext, RfdRevisionIdentifier, RfdWithContent, RfdWithoutContent}, permissions::RfdPermission, search::{MeiliSearchResult, SearchRequest}, util::response::{client_error, internal_error, unauthorized}, }; +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdPathParams { + /// The RFD number (examples: 1 or 123) + number: String, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdRevisionPathParams { + /// The RFD number (examples: 1 or 123) + number: String, + /// The revision id of the RFD + revision: TypedUuid, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdAttrPathParams { + /// The RFD number (examples: 1 or 123) + number: String, + /// An attribute that can be defined in an RFD document + attr: RfdAttrName, +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdRevisionAttrPathParams { + /// The RFD number (examples: 1 or 123) + number: String, + /// The revision id of the RFD + revision: TypedUuid, + /// An attribute that can be defined in an RFD document + attr: RfdAttrName, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub enum RfdAttrName { + Discussion, + Labels, + State, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "kebab-case")] +pub enum RfdAttr { + Discussion(String), + Labels(String), + State(RfdState), +} + +// Read Endpoints + /// List all available RFDs #[trace_request] #[endpoint { @@ -44,15 +95,408 @@ pub async fn list_rfds( list_rfds_op(ctx, &caller).await } -#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn list_rfds_op( - ctx: &RfdContext, - caller: &Caller, -) -> Result>, HttpError> { - let rfds = ctx.list_rfds(caller, None).await?; - Ok(HttpResponseOk(rfds)) +// Latest RFD revision endpoints + +/// Get the latest representation of an RFD's metadata +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_meta( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_meta_op(ctx, &caller, path.number, None).await +} + +/// Get the raw contents of the latest revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/raw", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_op(ctx, &caller, path.number, None).await +} + +/// Get the PDF locations of the latest revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/pdf", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_pdf( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_pdf_op(ctx, &caller, path.number, None).await +} + +/// Get the an attribute of the latest revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/attr/{attr}", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_attr( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_attr_op(ctx, &caller, path.number, None, path.attr).await +} + +/// Get the comments related to the latest revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/comments", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_comments( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + unimplemented!() +} + +// Specific RFD revision endpoints + +/// Get an RFD revision's metadata +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/revision/{revision}", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_revision_meta( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_meta_op(ctx, &caller, path.number, Some(path.revision.into())).await +} + +/// Get the raw contents of a revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/revision/{revision}/raw", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_revision( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_op(ctx, &caller, path.number, Some(path.revision.into())).await +} + +/// Get the PDF locations of a revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/revision/{revision}/pdf", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_revision_pdf( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_pdf_op(ctx, &caller, path.number, Some(path.revision.into())).await +} + +/// Get the an attribute of a revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/revision/{revision}/attr/{attr}", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_revision_attr( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_attr_op( + ctx, + &caller, + path.number, + Some(path.revision.into()), + path.attr, + ) + .await +} + +/// Get the comments related to a revision of a RFD +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd/{number}/revision/{revision}/comments", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn view_rfd_revision_comments( + rqctx: RequestContext, + path: Path, +) -> Result, HttpError> { + unimplemented!() +} + +#[derive(Debug, Deserialize, JsonSchema)] +pub struct RfdSearchQuery { + pub q: String, + pub limit: Option, + pub offset: Option, + pub highlight_pre_tag: Option, + pub highlight_post_tag: Option, + pub attributes_to_crop: Option, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct SearchResults { + hits: Vec, + query: String, + limit: Option, + offset: Option, +} + +// TODO: This should be a shared type across the api and processor, but it likely needs custom +// deserialization, serialization, and schema implementations +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct SearchResultHit { + pub hierarchy: [Option; 6], + pub hierarchy_radio: [Option; 6], + pub content: String, + pub object_id: String, + pub rfd_number: u64, + pub anchor: Option, + pub url: Option, + pub formatted: Option, +} + +#[derive(Debug, Deserialize, Serialize, JsonSchema)] +pub struct FormattedSearchResultHit { + pub hierarchy: [Option; 6], + pub hierarchy_radio: [Option; 6], + pub content: Option, + pub object_id: String, + pub rfd_number: u64, + pub anchor: Option, + pub url: Option, +} + +/// Search the RFD index and get a list of results +#[trace_request] +#[endpoint { + method = GET, + path = "/rfd-search", +}] +#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] +pub async fn search_rfds( + rqctx: RequestContext, + query: Query, +) -> Result, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + search_rfds_op(ctx, &caller, query.into_inner()).await +} + +// Read operation + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn list_rfds_op( + ctx: &RfdContext, + caller: &Caller, +) -> Result>, HttpError> { + let rfds = ctx.list_rfds(caller, None).await?; + Ok(HttpResponseOk(rfds)) +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn view_rfd_meta_op( + ctx: &RfdContext, + caller: &Caller, + number: String, + revision: Option, +) -> Result, HttpError> { + if let Ok(rfd_number) = number.parse::() { + Ok(HttpResponseOk( + ctx.view_rfd_meta(caller, rfd_number, revision).await?, + )) + } else { + Err(client_error( + ClientErrorStatusCode::BAD_REQUEST, + "Malformed RFD number", + )) + } +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn view_rfd_op( + ctx: &RfdContext, + caller: &Caller, + number: String, + revision: Option, +) -> Result, HttpError> { + if let Ok(rfd_number) = number.parse::() { + Ok(HttpResponseOk( + ctx.view_rfd(caller, rfd_number, revision).await?, + )) + } else { + Err(client_error( + ClientErrorStatusCode::BAD_REQUEST, + "Malformed RFD number", + )) + } +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn view_rfd_pdf_op( + ctx: &RfdContext, + caller: &Caller, + number: String, + revision: Option, +) -> Result, HttpError> { + unimplemented!() +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn view_rfd_attr_op( + ctx: &RfdContext, + caller: &Caller, + number: String, + revision: Option, + attr: RfdAttrName, +) -> Result, HttpError> { + if let Ok(rfd_number) = number.parse::() { + let rfd = ctx.view_rfd(caller, rfd_number, None).await?; + let content = match rfd.format { + ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(rfd.content)), + ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(rfd.content)), + }; + + extract_attr(&attr, &content).map(HttpResponseOk) + } else { + Err(client_error( + ClientErrorStatusCode::BAD_REQUEST, + "Malformed RFD number", + )) + } +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn view_rfd_comments_op( + ctx: &RfdContext, + caller: &Caller, + number: String, + revision: Option, +) -> Result, HttpError> { + unimplemented!() +} + +#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] +async fn search_rfds_op( + ctx: &RfdContext, + caller: &Caller, + query: RfdSearchQuery, +) -> Result, HttpError> { + // TODO: Move all of this into a ctx + + // Ensure that the user has the search permission before searching + if caller.can(&RfdPermission::SearchRfds) { + tracing::debug!("Fetching from remote search API"); + + // Transform the inbound query into a meilisearch request + let mut search_request: SearchRequest = query.into(); + + // Construct a meilisearch formatted filter. Either the caller has permission to search across + // all RFDs or they access to some smaller set. If we need to filter down the RFD list we + // construct a filter that will search across the RFDs the caller has direct access to as + // well as any RFDs that are marked as publicly accessible. + search_request.filter = if caller.can(&RfdPermission::GetRfdsAll) { + None + } else { + let mut filter = "public = true".to_string(); + + let allowed_rfds = caller + .allow_rfds() + .iter() + .map(|num| num.to_string()) + .collect::>() + .join(", "); + if allowed_rfds.len() > 0 { + filter = filter + &format!("OR rfd_number in [{}]", allowed_rfds); + } + + Some(filter) + }; + + // Pass the search request off to the meilisearch backend + let results = ctx + .search + .client + .search::(&search_request) + .await; + + tracing::debug!("Fetched results from remote search"); + + match results { + Ok(results) => { + let results = SearchResults { + hits: results + .hits + .into_iter() + .map(|hit| hit.into()) + .collect::>(), + query: results.query, + limit: results.limit, + offset: results.offset, + }; + + tracing::debug!(count = ?results.hits.len(), "Transformed search results"); + + Ok(HttpResponseOk(results)) + } + Err(err) => { + tracing::error!(?err, "Search request failed"); + Err(internal_error("Search failed".to_string())) + } + } + } else { + Err(unauthorized()) + } } +// Write Endpoints + #[derive(Debug, Deserialize, JsonSchema)] pub struct ReserveRfdBody { /// Title of the RFD @@ -94,46 +538,6 @@ async fn reserve_rfd_op( })) } -#[derive(Debug, Deserialize, JsonSchema)] -pub struct RfdPathParams { - /// The RFD number (examples: 1 or 123) - number: String, -} - -/// Get the latest representation of a RFD -#[trace_request] -#[endpoint { - method = GET, - path = "/rfd/{number}", -}] -#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn view_rfd( - rqctx: RequestContext, - path: Path, -) -> Result, HttpError> { - let ctx = rqctx.context(); - let caller = ctx.v_ctx().get_caller(&rqctx).await?; - view_rfd_op(ctx, &caller, path.into_inner().number).await -} - -#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn view_rfd_op( - ctx: &RfdContext, - caller: &Caller, - number: String, -) -> Result, HttpError> { - if let Ok(rfd_number) = number.parse::() { - Ok(HttpResponseOk( - ctx.view_rfd(caller, rfd_number, None).await?, - )) - } else { - Err(client_error( - ClientErrorStatusCode::BAD_REQUEST, - "Malformed RFD number", - )) - } -} - #[derive(Debug, Deserialize, JsonSchema)] pub struct RfdUpdateBody { /// Full Asciidoc document to store for this RFD @@ -146,7 +550,7 @@ pub struct RfdUpdateBody { #[trace_request] #[endpoint { method = POST, - path = "/rfd/{number}", + path = "/rfd/{number}/raw", }] pub async fn set_rfd_document( rqctx: RequestContext, @@ -230,68 +634,6 @@ async fn set_rfd_content_op( } } -#[derive(Debug, Deserialize, JsonSchema)] -pub struct RfdAttrPathParams { - number: String, - attr: RfdAttrName, -} - -#[derive(Debug, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "kebab-case")] -pub enum RfdAttrName { - Discussion, - Labels, - State, -} - -#[derive(Debug, Deserialize, Serialize, JsonSchema)] -#[serde(rename_all = "kebab-case")] -pub enum RfdAttr { - Discussion(String), - Labels(String), - State(RfdState), -} - -/// Get an attribute of a RFD -#[trace_request] -#[endpoint { - method = GET, - path = "/rfd/{number}/attr/{attr}", -}] -#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn view_rfd_attr( - rqctx: RequestContext, - path: Path, -) -> Result, HttpError> { - let ctx = rqctx.context(); - let caller = ctx.v_ctx().get_caller(&rqctx).await?; - let path = path.into_inner(); - view_rfd_attr_op(ctx, &caller, path.number, path.attr).await -} - -#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn view_rfd_attr_op( - ctx: &RfdContext, - caller: &Caller, - number: String, - attr: RfdAttrName, -) -> Result, HttpError> { - if let Ok(rfd_number) = number.parse::() { - let rfd = ctx.view_rfd(caller, rfd_number, None).await?; - let content = match rfd.format { - ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(rfd.content)), - ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(rfd.content)), - }; - - extract_attr(&attr, &content).map(HttpResponseOk) - } else { - Err(client_error( - ClientErrorStatusCode::BAD_REQUEST, - "Malformed RFD number", - )) - } -} - #[derive(Debug, Deserialize, Serialize, JsonSchema)] pub struct RfdAttrValue { /// Full value to set this attribute to in the existing RFD contents @@ -466,138 +808,6 @@ fn extract_attr(attr: &RfdAttrName, content: &RfdContent) -> Result, - pub offset: Option, - pub highlight_pre_tag: Option, - pub highlight_post_tag: Option, - pub attributes_to_crop: Option, -} - -/// Search the RFD index and get a list of results -#[trace_request] -#[endpoint { - method = GET, - path = "/rfd-search", -}] -#[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn search_rfds( - rqctx: RequestContext, - query: Query, -) -> Result, HttpError> { - let ctx = rqctx.context(); - let caller = ctx.v_ctx().get_caller(&rqctx).await?; - search_rfds_op(ctx, &caller, query.into_inner()).await -} - -#[derive(Debug, Deserialize, Serialize, JsonSchema)] -pub struct SearchResults { - hits: Vec, - query: String, - limit: Option, - offset: Option, -} - -// TODO: This should be a shared type across the api and processor, but it likely needs custom -// deserialization, serialization, and schema implementations -#[derive(Debug, Deserialize, Serialize, JsonSchema)] -pub struct SearchResultHit { - pub hierarchy: [Option; 6], - pub hierarchy_radio: [Option; 6], - pub content: String, - pub object_id: String, - pub rfd_number: u64, - pub anchor: Option, - pub url: Option, - pub formatted: Option, -} - -#[derive(Debug, Deserialize, Serialize, JsonSchema)] -pub struct FormattedSearchResultHit { - pub hierarchy: [Option; 6], - pub hierarchy_radio: [Option; 6], - pub content: Option, - pub object_id: String, - pub rfd_number: u64, - pub anchor: Option, - pub url: Option, -} - -#[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn search_rfds_op( - ctx: &RfdContext, - caller: &Caller, - query: RfdSearchQuery, -) -> Result, HttpError> { - // TODO: Move all of this into a ctx - - // Ensure that the user has the search permission before searching - if caller.can(&RfdPermission::SearchRfds) { - tracing::debug!("Fetching from remote search API"); - - // Transform the inbound query into a meilisearch request - let mut search_request: SearchRequest = query.into(); - - // Construct a meilisearch formatted filter. Either the caller has permission to search across - // all RFDs or they access to some smaller set. If we need to filter down the RFD list we - // construct a filter that will search across the RFDs the caller has direct access to as - // well as any RFDs that are marked as publicly accessible. - search_request.filter = if caller.can(&RfdPermission::GetRfdsAll) { - None - } else { - let mut filter = "public = true".to_string(); - - let allowed_rfds = caller - .allow_rfds() - .iter() - .map(|num| num.to_string()) - .collect::>() - .join(", "); - if allowed_rfds.len() > 0 { - filter = filter + &format!("OR rfd_number in [{}]", allowed_rfds); - } - - Some(filter) - }; - - // Pass the search request off to the meilisearch backend - let results = ctx - .search - .client - .search::(&search_request) - .await; - - tracing::debug!("Fetched results from remote search"); - - match results { - Ok(results) => { - let results = SearchResults { - hits: results - .hits - .into_iter() - .map(|hit| hit.into()) - .collect::>(), - query: results.query, - limit: results.limit, - offset: results.offset, - }; - - tracing::debug!(count = ?results.hits.len(), "Transformed search results"); - - Ok(HttpResponseOk(results)) - } - Err(err) => { - tracing::error!(?err, "Search request failed"); - Err(internal_error("Search failed".to_string())) - } - } - } else { - Err(unauthorized()) - } -} - #[derive(Debug, Deserialize, JsonSchema)] pub struct RfdVisibility { /// @@ -1030,12 +1240,12 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfdsAll])); - let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0123".to_string()) + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0123".to_string(), None) .await .unwrap(); assert_eq!(123, rfd.rfd_number); - let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0456".to_string()) + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0456".to_string(), None) .await .unwrap(); assert_eq!(456, rfd.rfd_number); @@ -1059,12 +1269,12 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::from(vec![RfdPermission::GetRfd(123)])); - let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0123".to_string()) + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0123".to_string(), None) .await .unwrap(); assert_eq!(123, rfd.rfd_number); - let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0456".to_string()) + let HttpResponseOk(rfd) = view_rfd_op(&ctx, &caller, "0456".to_string(), None) .await .unwrap(); assert_eq!(456, rfd.rfd_number); @@ -1087,7 +1297,7 @@ mod tests { let ctx = ctx().await; let caller = Caller::from(Permissions::::new()); - let result = view_rfd_op(&ctx, &caller, "0123".to_string()).await; + let result = view_rfd_op(&ctx, &caller, "0123".to_string(), None).await; match result { Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), @@ -1117,7 +1327,7 @@ mod tests { let ctx = ctx().await; let caller = ctx.v_ctx().builtin_unauthenticated_caller(); - let result = view_rfd_op(&ctx, &caller, "0123".to_string()).await; + let result = view_rfd_op(&ctx, &caller, "0123".to_string(), None).await; match result { Err(err) => assert_eq!(StatusCode::NOT_FOUND, err.status_code), Ok(response) => panic!( diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index ddb726a3..18080922 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -15,10 +15,12 @@ use v_api::{inject_endpoints, v_system_endpoints}; use crate::{ context::RfdContext, endpoints::{ - meta::view_rfd_meta, rfd::{ discuss_rfd, list_rfds, publish_rfd, reserve_rfd, search_rfds, set_rfd_attr, set_rfd_content, set_rfd_document, update_rfd_visibility, view_rfd, view_rfd_attr, + view_rfd_comments, view_rfd_meta, view_rfd_pdf, view_rfd_revision, + view_rfd_revision_attr, view_rfd_revision_comments, view_rfd_revision_meta, + view_rfd_revision_pdf, }, webhook::github_webhook, }, @@ -76,17 +78,37 @@ pub fn server( // RFDs api.register(list_rfds) .expect("Failed to register endpoint"); - api.register(view_rfd).expect("Failed to register endpoint"); + api.register(view_rfd_meta) .expect("Failed to register endpoint"); + api.register(view_rfd).expect("Failed to register endpoint"); + api.register(view_rfd_pdf) + .expect("Failed to register endpoint"); + api.register(view_rfd_attr) + .expect("Failed to register endpoint"); + api.register(view_rfd_comments) + .expect("Failed to register endpoint"); + + api.register(view_rfd_revision_meta) + .expect("Failed to register endpoint"); + api.register(view_rfd_revision) + .expect("Failed to register endpoint"); + api.register(view_rfd_revision_pdf) + .expect("Failed to register endpoint"); + api.register(view_rfd_revision_attr) + .expect("Failed to register endpoint"); + api.register(view_rfd_revision_comments) + .expect("Failed to register endpoint"); + + api.register(search_rfds) + .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(view_rfd_attr) - .expect("Failed to register endpoint"); api.register(set_rfd_attr) .expect("Failed to register endpoint"); api.register(discuss_rfd) @@ -95,8 +117,6 @@ pub fn server( .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-model/src/lib.rs b/rfd-model/src/lib.rs index 0dc53323..d897d9f5 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -111,6 +111,17 @@ impl From<(RfdModel, RfdRevisionMetaModel)> for RfdMeta { } } +impl From for NewRfd { + fn from(value: RfdMeta) -> Self { + Self { + id: value.id, + rfd_number: value.rfd_number, + link: value.link, + visibility: value.visibility, + } + } +} + #[derive(JsonSchema)] pub enum RfdRevisionId {} impl TypedUuidKind for RfdRevisionId { diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index 3546d6bc..a583e06b 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -12,8 +12,8 @@ use std::fmt::Debug; use v_model::storage::{ListPagination, StoreError}; use crate::{ - schema_ext::PdfSource, Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdMeta, - RfdPdf, RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, + schema_ext::PdfSource, CommitSha, Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, + RfdMeta, RfdPdf, RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, }; #[cfg(feature = "mock")] @@ -50,7 +50,7 @@ pub struct RfdFilter { pub id: Option>>, pub revision: Option>>, pub rfd_number: Option>, - pub sha: Option>, + pub commit_sha: Option>, pub public: Option, pub deleted: bool, } @@ -71,8 +71,8 @@ impl RfdFilter { self } - pub fn sha(mut self, sha: Option>) -> Self { - self.sha = sha; + pub fn commit_sha(mut self, commit_sha: Option>) -> Self { + self.commit_sha = commit_sha; self } diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index d4212de5..bb954cb4 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -72,7 +72,7 @@ impl RfdStore for PostgresStore { id, revision, rfd_number, - sha, + commit_sha, public, deleted, } = filter; @@ -94,8 +94,10 @@ impl RfdStore for PostgresStore { predicates.push(Box::new(rfd::rfd_number.eq_any(rfd_number))); } - if let Some(sha) = sha { - predicates.push(Box::new(rfd_revision::sha.eq_any(sha))); + if let Some(commit_sha) = commit_sha { + predicates.push(Box::new( + rfd_revision::commit_sha.eq_any(commit_sha.into_iter().map(|sha| sha.0)), + )); } if let Some(public) = public { @@ -212,7 +214,7 @@ impl RfdMetaStore for PostgresStore { id, revision, rfd_number, - sha, + commit_sha, public, deleted, } = filter; @@ -234,8 +236,10 @@ impl RfdMetaStore for PostgresStore { predicates.push(Box::new(rfd::rfd_number.eq_any(rfd_number))); } - if let Some(sha) = sha { - predicates.push(Box::new(rfd_revision::sha.eq_any(sha))); + if let Some(commit_sha) = commit_sha { + predicates.push(Box::new( + rfd_revision::commit_sha.eq_any(commit_sha.into_iter().map(|sha| sha.0)), + )); } if let Some(public) = public { From 6b9b132f3c710ef27db9f01eea79347bf3d008b3 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 13:37:03 -0600 Subject: [PATCH 07/17] Split out pdfs --- rfd-api/src/context.rs | 25 +++++++++++++++++++++++-- rfd-api/src/endpoints/rfd.rs | 19 ++++++++++++++----- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index a582ddf1..d60e0147 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -17,8 +17,8 @@ use rfd_data::{ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - storage::{JobStore, RfdFilter, RfdMetaStore, RfdStorage, RfdStore}, - CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdRevision, RfdRevisionId, + storage::{JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdStorage, RfdStore}, + CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdPdf, RfdRevision, RfdRevisionId, }; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, @@ -491,6 +491,27 @@ impl RfdContext { self.view_rfd_revision(caller, rfd_number, None).await } + #[instrument(skip(self, caller))] + pub async fn view_rfd_pdfs( + &self, + caller: &Caller, + rfd_number: i32, + revision: Option, + ) -> ResourceResult, StoreError> { + let rfd = self.get_rfd_meta(caller, rfd_number, revision).await?; + let pdfs = RfdPdfStore::list( + &*self.storage, + vec![RfdPdfFilter::default() + .rfd(Some(vec![rfd.id])) + .rfd_revision(Some(vec![rfd.content.id]))], + &ListPagination::unlimited(), + ) + .await + .to_resource_result()?; + + Ok(pdfs) + } + #[instrument(skip(self, caller, content))] pub async fn update_rfd_content( &self, diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 372ffa18..4b766530 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -13,7 +13,7 @@ use rfd_data::{ }; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - Rfd, RfdRevisionId, + Rfd, RfdPdf, RfdRevisionId, }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -141,7 +141,7 @@ pub async fn view_rfd( pub async fn view_rfd_pdf( rqctx: RequestContext, path: Path, -) -> Result, HttpError> { +) -> Result>, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); @@ -225,7 +225,7 @@ pub async fn view_rfd_revision( pub async fn view_rfd_revision_pdf( rqctx: RequestContext, path: Path, -) -> Result, HttpError> { +) -> Result>, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); @@ -384,8 +384,17 @@ async fn view_rfd_pdf_op( caller: &Caller, number: String, revision: Option, -) -> Result, HttpError> { - unimplemented!() +) -> Result>, HttpError> { + if let Ok(rfd_number) = number.parse::() { + Ok(HttpResponseOk( + ctx.view_rfd_pdfs(caller, rfd_number, revision).await?, + )) + } else { + Err(client_error( + ClientErrorStatusCode::BAD_REQUEST, + "Malformed RFD number", + )) + } } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] From 182c2b698103dec2bed5793b0c80c5e23a73092d Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 15:39:59 -0600 Subject: [PATCH 08/17] Add initial model sketches --- .../down.sql | 1 + .../2024-12-21-204322_add_rfd_comments/up.sql | 44 +++++++++++++ rfd-model/src/db.rs | 45 +++++++++++++- rfd-model/src/lib.rs | 61 +++++++++++++++++++ rfd-model/src/schema.rs | 54 +++++++++++++++- 5 files changed, 203 insertions(+), 2 deletions(-) create mode 100644 rfd-model/migrations/2024-12-21-204322_add_rfd_comments/down.sql create mode 100644 rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql diff --git a/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/down.sql b/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/down.sql new file mode 100644 index 00000000..d9a93fe9 --- /dev/null +++ b/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/down.sql @@ -0,0 +1 @@ +-- This file should undo anything in `up.sql` diff --git a/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql b/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql new file mode 100644 index 00000000..cb521f19 --- /dev/null +++ b/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql @@ -0,0 +1,44 @@ +CREATE TABLE rfd_comment_user ( + id UUID PRIMARY KEY, + github_user_id INTEGER NOT NULL, + github_user_node_id VARCHAR NOT NULL, + github_user_username VARCHAR, + github_user_avatar_url VARCHAR, + github_user_type VARCHAR NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + deleted_at TIMESTAMPTZ, + + UNIQUE(github_user_id), + UNIQUE(github_user_node_id) +); + +CREATE TABLE rfd_comment ( + id UUID PRIMARY KEY, + rfd_id UUID REFERENCES rfd (id) NOT NULL, + comment_user_id UUID REFERENCES rfd_comment_user(id) NOT NULL, + external_id INTEGER NOT NULL, + node_id -> VARCHAR NOT NULL, + discussion_number INTEGER, + diff_hunk -> VARCHAR NOT NULL, + path -> VARCHAR NOT NULL, + body -> VARCHAR, + commit_id -> VARCHAR NOT NULL, + original_commit_id -> VARCHAR NOT NULL, + line INTEGER, + original_line INTEGER, + start_line INTEGER, + original_start_line INTEGER, + side -> VARCHAR, + start_side -> VARCHAR, + subject VARCHAR NOT NULL, + in_reply_to INTEGER, + comment_created_at TIMESTAMPTZ, + comment_updated_at TIMESTAMPTZ, + created_at -> TIMESTAMPTZ NOT NULL, + updated_at -> TIMESTAMPTZ NOT NULL, + deleted_at -> TIMESTAMPTZ, + + UNIQUE(external_id), + UNIQUE(node_id) +); diff --git a/rfd-model/src/db.rs b/rfd-model/src/db.rs index 92aac45c..16d37f0a 100644 --- a/rfd-model/src/db.rs +++ b/rfd-model/src/db.rs @@ -9,7 +9,7 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::{ - schema::{job, rfd, rfd_pdf, rfd_revision}, + schema::{job, rfd, rfd_comment, rfd_comment_user, rfd_pdf, rfd_revision}, schema_ext::{ContentFormat, PdfSource, Visibility}, }; @@ -76,3 +76,46 @@ pub struct JobModel { pub created_at: DateTime, pub started_at: Option>, } + +#[derive(Debug, Deserialize, Serialize, Queryable, Insertable)] +#[diesel(table_name = rfd_comment_user)] +pub struct RfdCommentUserModel { + pub id: Uuid, + pub github_user_id: i32, + pub github_user_node_id: String, + pub github_user_username: Option, + pub github_user_avatar_url: Option, + pub github_user_type: String, + pub created_at: DateTime, + pub updated_at: DateTime, + pub deleted_at: Option>, +} + +#[derive(Debug, Deserialize, Serialize, Queryable, Insertable)] +#[diesel(table_name = rfd_comment)] +pub struct RfdCommentModel { + pub id: Uuid, + pub rfd_id: Uuid, + pub comment_user_id: Uuid, + pub external_id: Option, + pub node_id: String, + pub discussion_number: Option, + pub diff_hunk: String, + pub path: String, + pub body: Option, + pub commit_id: String, + pub original_commit_id: String, + pub line: Option, + pub original_line: Option, + pub start_line: Option, + pub original_start_line: Option, + pub side: Option, + pub start_side: Option, + pub subject: String, + pub in_reply_to: Option, + pub comment_created_at: Option>, + pub comment_updated_at: Option>, + pub created_at: DateTime, + pub updated_at: DateTime, + pub deleted_at: Option>, +} diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index d897d9f5..5cb9378a 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -299,3 +299,64 @@ impl Display for InvalidValueError { write!(f, "{} has an invalid value: {}", self.field, self.error) } } + +#[derive(JsonSchema)] +pub enum RfdCommentUserId {} +impl TypedUuidKind for RfdCommentUserId { + fn tag() -> TypedUuidTag { + const TAG: TypedUuidTag = TypedUuidTag::new("rfd-comment-user"); + TAG + } +} + +#[partial(NewRfdCommentUser)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct RfdCommentUser { + pub id: TypedUuid, + pub github_user_id: i32, + pub github_user_node_id: String, + pub github_user_username: Option, + pub github_user_avatar_url: Option, + pub github_user_type: String, + pub created_at: DateTime, + pub updated_at: DateTime, + pub deleted_at: Option>, +} + +#[derive(JsonSchema)] +pub enum RfdCommentId {} +impl TypedUuidKind for RfdCommentId { + fn tag() -> TypedUuidTag { + const TAG: TypedUuidTag = TypedUuidTag::new("rfd-comment"); + TAG + } +} + +#[partial(NewRfdComment)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct RfdComment { + pub id: TypedUuid, + pub rfd_id: TypedUuid, + pub comment_user_id: TypedUuid, + pub external_id: Option, + pub node_id: String, + pub discussion_number: Option, + pub diff_hunk: String, + pub path: String, + pub body: Option, + pub commit_id: String, + pub original_commit_id: String, + pub line: Option, + pub original_line: Option, + pub start_line: Option, + pub original_start_line: Option, + pub side: Option, + pub start_side: Option, + pub subject: String, + pub in_reply_to: Option, + pub comment_created_at: Option>, + pub comment_updated_at: Option>, + pub created_at: DateTime, + pub updated_at: DateTime, + pub deleted_at: Option>, +} diff --git a/rfd-model/src/schema.rs b/rfd-model/src/schema.rs index 8eb4b499..c7510e2b 100644 --- a/rfd-model/src/schema.rs +++ b/rfd-model/src/schema.rs @@ -87,8 +87,60 @@ diesel::table! { } } +diesel::table! { + rfd_comment_user (id) { + id -> Uuid, + github_user_id -> Integer, + github_user_node_id -> Varchar, + github_user_username -> Nullable, + github_user_avatar_url -> Varchar, + github_user_type -> Varchar, + created_at -> Timestamptz, + updated_at -> Timestamptz, + deleted_at -> Nullable, + } +} + +diesel::table! { + rfd_comment (id) { + id -> Uuid, + rfd_id -> Uuid, + comment_user_id -> Uuid, + external_id -> Integer, + node_id -> Varchar, + discussion_number -> Nullable, + diff_hunk -> Varchar, + path -> Varchar, + body -> Nullable, + commit_id -> Varchar, + original_commit_id -> Varchar, + line -> Nullable, + original_line -> Nullable, + start_line -> Nullable, + original_start_line -> Nullable, + side -> Nullable, + start_side -> Nullable, + subject -> Varchar, + in_reply_to -> Nullable, + comment_created_at -> Nullable, + comment_updated_at -> Nullable, + created_at -> Timestamptz, + updated_at -> Timestamptz, + deleted_at -> Nullable, + } +} + diesel::joinable!(rfd_pdf -> rfd (rfd_id)); diesel::joinable!(rfd_pdf -> rfd_revision (rfd_revision_id)); diesel::joinable!(rfd_revision -> rfd (rfd_id)); +diesel::joinable!(rfd_comment -> rfd (rfd_id)); +diesel::joinable!(rfd_comment -> rfd_comment_user (comment_user_id)); -diesel::allow_tables_to_appear_in_same_query!(job, rfd, rfd_pdf, rfd_revision,); +diesel::allow_tables_to_appear_in_same_query!( + job, + rfd, + rfd_pdf, + rfd_revision, + rfd_comment, + rfd_comment_user +); From 7489d1ab3879073bc1ea558fb051e6e42d276610 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 16:01:21 -0600 Subject: [PATCH 09/17] Sketch out wiring --- rfd-api/src/context.rs | 12 ++++++++- rfd-api/src/endpoints/rfd.rs | 30 +++++++++++++++------ rfd-model/src/lib.rs | 2 +- rfd-model/src/storage/mod.rs | 44 +++++++++++++++++++++++++++++-- rfd-model/src/storage/postgres.rs | 39 ++++++++++++++++++++++++--- 5 files changed, 111 insertions(+), 16 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index d60e0147..9ca3f124 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -18,7 +18,8 @@ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, storage::{JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdStorage, RfdStore}, - CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdPdf, RfdRevision, RfdRevisionId, + CommitSha, FileSha, Job, NewJob, Rfd, RfdComment, RfdId, RfdMeta, RfdPdf, RfdRevision, + RfdRevisionId, }; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, @@ -512,6 +513,15 @@ impl RfdContext { Ok(pdfs) } + pub async fn view_rfd_comments( + &self, + caller: &Caller, + rfd_number: i32, + revision: Option, + ) -> ResourceResult, StoreError> { + unimplemented!() + } + #[instrument(skip(self, caller, content))] pub async fn update_rfd_content( &self, diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 4b766530..600c73b5 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -13,7 +13,7 @@ use rfd_data::{ }; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - Rfd, RfdPdf, RfdRevisionId, + Rfd, RfdComment, RfdPdf, RfdRevisionId, }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -175,8 +175,11 @@ pub async fn view_rfd_attr( pub async fn view_rfd_comments( rqctx: RequestContext, path: Path, -) -> Result, HttpError> { - unimplemented!() +) -> Result>, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_comments_op(ctx, &caller, path.number, None).await } // Specific RFD revision endpoints @@ -266,8 +269,11 @@ pub async fn view_rfd_revision_attr( pub async fn view_rfd_revision_comments( rqctx: RequestContext, path: Path, -) -> Result, HttpError> { - unimplemented!() +) -> Result>, HttpError> { + let ctx = rqctx.context(); + let caller = ctx.v_ctx().get_caller(&rqctx).await?; + let path = path.into_inner(); + view_rfd_comments_op(ctx, &caller, path.number, Some(path.revision.into())).await } #[derive(Debug, Deserialize, JsonSchema)] @@ -406,7 +412,7 @@ async fn view_rfd_attr_op( attr: RfdAttrName, ) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { - let rfd = ctx.view_rfd(caller, rfd_number, None).await?; + let rfd = ctx.view_rfd(caller, rfd_number, revision).await?; let content = match rfd.format { ContentFormat::Asciidoc => RfdContent::Asciidoc(RfdAsciidoc::new(rfd.content)), ContentFormat::Markdown => RfdContent::Markdown(RfdMarkdown::new(rfd.content)), @@ -427,8 +433,16 @@ async fn view_rfd_comments_op( caller: &Caller, number: String, revision: Option, -) -> Result, HttpError> { - unimplemented!() +) -> Result>, HttpError> { + if let Ok(rfd_number) = number.parse::() { + let comments = ctx.view_rfd_comments(caller, rfd_number, revision).await?; + Ok(HttpResponseOk(comments)) + } else { + Err(client_error( + ClientErrorStatusCode::BAD_REQUEST, + "Malformed RFD number", + )) + } } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index 5cb9378a..dd264a06 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -337,7 +337,7 @@ impl TypedUuidKind for RfdCommentId { pub struct RfdComment { pub id: TypedUuid, pub rfd_id: TypedUuid, - pub comment_user_id: TypedUuid, + pub comment_user: RfdCommentUser, pub external_id: Option, pub node_id: String, pub discussion_number: Option, diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index a583e06b..58177122 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -12,8 +12,9 @@ use std::fmt::Debug; use v_model::storage::{ListPagination, StoreError}; use crate::{ - schema_ext::PdfSource, CommitSha, Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, - RfdMeta, RfdPdf, RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, + schema_ext::PdfSource, CommitSha, Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, + NewRfdPdf, NewRfdRevision, Rfd, RfdComment, RfdCommentId, RfdCommentUser, RfdCommentUserId, + RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, }; #[cfg(feature = "mock")] @@ -308,3 +309,42 @@ pub trait JobStore { async fn start(&self, id: i32) -> Result, StoreError>; async fn complete(&self, id: i32) -> Result, StoreError>; } + +#[cfg_attr(feature = "mock", automock)] +#[async_trait] +pub trait RfdCommentUserStore { + async fn upsert( + &self, + new_rfd_comment_user: NewRfdCommentUser, + ) -> Result; +} + +#[derive(Debug, Default)] +pub struct RfdCommentFilter { + pub rfd: Option>>, + pub user: Option>>, +} + +impl RfdCommentFilter { + pub fn rfd(mut self, rfd: Option>>) -> Self { + self.rfd = rfd; + self + } + + pub fn user(mut self, user: Option>>) -> Self { + self.user = user; + self + } +} + +#[cfg_attr(feature = "mock", automock)] +#[async_trait] +pub trait RfdCommentStore { + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError>; + async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result; + async fn delete(&self, id: &TypedUuid) -> Result, StoreError>; +} diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index bb954cb4..05a6953d 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -23,13 +23,15 @@ use crate::{ schema::{job, rfd, rfd_pdf, rfd_revision}, schema_ext::Visibility, storage::StoreError, - Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdMeta, RfdPdf, RfdPdfId, - RfdRevision, RfdRevisionId, RfdRevisionMeta, + Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdRevision, Rfd, + RfdComment, RfdCommentId, RfdCommentUser, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdRevision, + RfdRevisionId, RfdRevisionMeta, }; use super::{ - JobFilter, JobStore, ListPagination, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, - RfdRevisionFilter, RfdRevisionMetaStore, RfdRevisionStore, RfdStore, + JobFilter, JobStore, ListPagination, RfdCommentFilter, RfdCommentStore, RfdCommentUserStore, + RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, + RfdRevisionStore, RfdStore, }; #[async_trait] @@ -748,6 +750,35 @@ impl JobStore for PostgresStore { } } +#[async_trait] +impl RfdCommentUserStore for PostgresStore { + async fn upsert( + &self, + new_rfd_comment_user: NewRfdCommentUser, + ) -> Result { + unimplemented!() + } +} + +#[async_trait] +impl RfdCommentStore for PostgresStore { + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + unimplemented!() + } + + async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result { + unimplemented!() + } + + async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { + unimplemented!() + } +} + fn flatten_predicates( predicates: Vec>>>, ) -> Option>> From e7726ed96d9427281df50ccfb8ad85646cdb2415 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 16:17:46 -0600 Subject: [PATCH 10/17] More wiring --- rfd-api/src/context.rs | 17 +++++++-- rfd-model/src/storage/mock.rs | 58 ++++++++++++++++++++++++++++--- rfd-model/src/storage/mod.rs | 23 ++++++------ rfd-model/src/storage/postgres.rs | 2 +- 4 files changed, 81 insertions(+), 19 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 9ca3f124..df317f60 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -17,7 +17,10 @@ use rfd_data::{ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - storage::{JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdStorage, RfdStore}, + storage::{ + JobStore, RfdCommentFilter, RfdCommentStore, RfdFilter, RfdMetaStore, RfdPdfFilter, + RfdPdfStore, RfdStorage, RfdStore, + }, CommitSha, FileSha, Job, NewJob, Rfd, RfdComment, RfdId, RfdMeta, RfdPdf, RfdRevision, RfdRevisionId, }; @@ -519,7 +522,17 @@ impl RfdContext { rfd_number: i32, revision: Option, ) -> ResourceResult, StoreError> { - unimplemented!() + let rfd = self.get_rfd_meta(caller, rfd_number, revision).await?; + let comments = RfdCommentStore::list( + &*self.storage, + vec![RfdCommentFilter::default() + .rfd(Some(vec![rfd.id])) + .comment_created_before(Some(rfd.content.committed_at))], + &ListPagination::unlimited(), + ) + .await + .to_resource_result()?; + Ok(comments) } #[instrument(skip(self, caller, content))] diff --git a/rfd-model/src/storage/mock.rs b/rfd-model/src/storage/mock.rs index cf3f829c..bbc48e39 100644 --- a/rfd-model/src/storage/mock.rs +++ b/rfd-model/src/storage/mock.rs @@ -8,14 +8,17 @@ use std::sync::Arc; use v_model::storage::StoreError; use crate::{ - Job, NewJob, NewRfd, NewRfdPdf, NewRfdRevision, Rfd, RfdId, RfdMeta, RfdPdf, RfdPdfId, - RfdRevision, RfdRevisionId, RfdRevisionMeta, + Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdRevision, Rfd, + RfdComment, RfdCommentId, RfdCommentUser, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdRevision, + RfdRevisionId, RfdRevisionMeta, }; use super::{ - JobFilter, JobStore, ListPagination, MockJobStore, MockRfdMetaStore, MockRfdPdfStore, - MockRfdRevisionMetaStore, MockRfdRevisionStore, MockRfdStore, RfdFilter, RfdMetaStore, - RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, RfdRevisionStore, RfdStore, + JobFilter, JobStore, ListPagination, MockJobStore, MockRfdCommentStore, + MockRfdCommentUserStore, MockRfdMetaStore, MockRfdPdfStore, MockRfdRevisionMetaStore, + MockRfdRevisionStore, MockRfdStore, RfdCommentFilter, RfdCommentStore, RfdCommentUserStore, + RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, + RfdRevisionStore, RfdStore, }; pub struct MockStorage { @@ -25,6 +28,8 @@ pub struct MockStorage { pub rfd_revision_meta_store: Option>, pub rfd_pdf_store: Option>, pub job_store: Option>, + pub rfd_comment_user_store: Option>, + pub rfd_comment_store: Option>, } impl MockStorage { @@ -36,6 +41,8 @@ impl MockStorage { rfd_revision_meta_store: None, rfd_pdf_store: None, job_store: None, + rfd_comment_user_store: None, + rfd_comment_store: None, } } } @@ -234,3 +241,44 @@ impl JobStore for MockStorage { self.job_store.as_ref().unwrap().complete(id).await } } + +#[async_trait] +impl RfdCommentUserStore for MockStorage { + async fn upsert( + &self, + new_rfd_comment_user: NewRfdCommentUser, + ) -> Result { + self.rfd_comment_user_store + .as_ref() + .unwrap() + .upsert(new_rfd_comment_user) + .await + } +} + +#[async_trait] +impl RfdCommentStore for MockStorage { + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_comment_store + .as_ref() + .unwrap() + .list(filters, pagination) + .await + } + + async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result { + self.rfd_comment_store + .as_ref() + .unwrap() + .upsert(new_rfd_comment) + .await + } + + async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { + self.rfd_comment_store.as_ref().unwrap().delete(id).await + } +} diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index 58177122..ac568389 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -4,6 +4,7 @@ pub use async_bb8_diesel::{ConnectionError, PoolError}; use async_trait::async_trait; +use chrono::{DateTime, Utc}; pub use diesel::result::Error as DbError; #[cfg(feature = "mock")] use mockall::automock; @@ -28,6 +29,8 @@ pub trait RfdStorage: + RfdRevisionMetaStore + RfdPdfStore + JobStore + + RfdCommentUserStore + + RfdCommentStore + Send + Sync + 'static @@ -40,6 +43,8 @@ impl RfdStorage for T where + RfdRevisionMetaStore + RfdPdfStore + JobStore + + RfdCommentUserStore + + RfdCommentStore + Send + Sync + 'static @@ -175,11 +180,6 @@ pub trait RfdRevisionStore { filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; - // async fn list_unique_rfd( - // &self, - // filters: Vec, - // pagination: &ListPagination, - // ) -> Result, StoreError>; async fn upsert(&self, new_revision: NewRfdRevision) -> Result; async fn delete( &self, @@ -200,11 +200,6 @@ pub trait RfdRevisionMetaStore { filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; - // async fn list_unique_rfd( - // &self, - // filter: RfdRevisionFilter, - // pagination: &ListPagination, - // ) -> Result, StoreError>; } #[derive(Debug, Default)] @@ -323,6 +318,7 @@ pub trait RfdCommentUserStore { pub struct RfdCommentFilter { pub rfd: Option>>, pub user: Option>>, + pub comment_created_before: Option>, } impl RfdCommentFilter { @@ -335,6 +331,11 @@ impl RfdCommentFilter { self.user = user; self } + + pub fn comment_created_before(mut self, comment_created_before: Option>) -> Self { + self.comment_created_before = comment_created_before; + self + } } #[cfg_attr(feature = "mock", automock)] @@ -344,7 +345,7 @@ pub trait RfdCommentStore { &self, filters: Vec, pagination: &ListPagination, - ) -> Result, StoreError>; + ) -> Result, StoreError>; async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result; async fn delete(&self, id: &TypedUuid) -> Result, StoreError>; } diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 05a6953d..5f984d94 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -766,7 +766,7 @@ impl RfdCommentStore for PostgresStore { &self, filters: Vec, pagination: &ListPagination, - ) -> Result, StoreError> { + ) -> Result, StoreError> { unimplemented!() } From b23a23a20c07d4a6bdbbd90ca1c70ce4040819d6 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 17:36:37 -0600 Subject: [PATCH 11/17] Start filling in models --- rfd-model/src/db.rs | 2 +- rfd-model/src/lib.rs | 61 ++++++++++++- rfd-model/src/schema.rs | 2 +- rfd-model/src/storage/mock.rs | 6 +- rfd-model/src/storage/mod.rs | 9 +- rfd-model/src/storage/postgres.rs | 138 ++++++++++++++++++++++++++++-- 6 files changed, 204 insertions(+), 14 deletions(-) diff --git a/rfd-model/src/db.rs b/rfd-model/src/db.rs index 16d37f0a..2a09bf5e 100644 --- a/rfd-model/src/db.rs +++ b/rfd-model/src/db.rs @@ -97,7 +97,7 @@ pub struct RfdCommentModel { pub id: Uuid, pub rfd_id: Uuid, pub comment_user_id: Uuid, - pub external_id: Option, + pub external_id: i32, pub node_id: String, pub discussion_number: Option, pub diff_hunk: String, diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index dd264a06..9ce2efbf 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -3,7 +3,10 @@ // file, You can obtain one at https://mozilla.org/MPL/2.0/. use chrono::{DateTime, Utc}; -use db::{JobModel, RfdModel, RfdPdfModel, RfdRevisionMetaModel, RfdRevisionModel}; +use db::{ + JobModel, RfdCommentModel, RfdCommentUserModel, RfdModel, RfdPdfModel, RfdRevisionMetaModel, + RfdRevisionModel, +}; use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag}; use partial_struct::partial; use schema_ext::{ContentFormat, PdfSource, Visibility}; @@ -318,11 +321,30 @@ pub struct RfdCommentUser { pub github_user_username: Option, pub github_user_avatar_url: Option, pub github_user_type: String, + #[partial(NewRfdComment(skip))] pub created_at: DateTime, + #[partial(NewRfdComment(skip))] pub updated_at: DateTime, + #[partial(NewRfdComment(skip))] pub deleted_at: Option>, } +impl From for RfdCommentUser { + fn from(value: RfdCommentUserModel) -> Self { + Self { + id: TypedUuid::from_untyped_uuid(value.id), + github_user_id: value.github_user_id, + github_user_node_id: value.github_user_node_id, + github_user_username: value.github_user_username, + github_user_avatar_url: value.github_user_avatar_url, + github_user_type: value.github_user_type, + created_at: value.created_at, + updated_at: value.updated_at, + deleted_at: value.deleted_at, + } + } +} + #[derive(JsonSchema)] pub enum RfdCommentId {} impl TypedUuidKind for RfdCommentId { @@ -337,8 +359,9 @@ impl TypedUuidKind for RfdCommentId { pub struct RfdComment { pub id: TypedUuid, pub rfd_id: TypedUuid, + #[partial(NewRfdComment(retype = TypedUuid))] pub comment_user: RfdCommentUser, - pub external_id: Option, + pub external_id: i32, pub node_id: String, pub discussion_number: Option, pub diff_hunk: String, @@ -356,7 +379,41 @@ pub struct RfdComment { pub in_reply_to: Option, pub comment_created_at: Option>, pub comment_updated_at: Option>, + #[partial(NewRfdComment(skip))] pub created_at: DateTime, + #[partial(NewRfdComment(skip))] pub updated_at: DateTime, + #[partial(NewRfdComment(skip))] pub deleted_at: Option>, } + +impl From<(RfdCommentModel, RfdCommentUserModel)> for RfdComment { + fn from((comment, user): (RfdCommentModel, RfdCommentUserModel)) -> Self { + Self { + id: TypedUuid::from_untyped_uuid(comment.id), + rfd_id: TypedUuid::from_untyped_uuid(comment.rfd_id), + comment_user: user.into(), + external_id: comment.external_id, + node_id: comment.node_id, + discussion_number: comment.discussion_number, + diff_hunk: comment.diff_hunk, + path: comment.path, + body: comment.body, + commit_id: comment.commit_id, + original_commit_id: comment.original_commit_id, + line: comment.line, + original_line: comment.original_line, + start_line: comment.start_line, + original_start_line: comment.original_start_line, + side: comment.side, + start_side: comment.start_side, + subject: comment.subject, + in_reply_to: comment.in_reply_to, + comment_created_at: comment.comment_created_at, + comment_updated_at: comment.comment_updated_at, + created_at: comment.created_at, + updated_at: comment.updated_at, + deleted_at: comment.deleted_at, + } + } +} diff --git a/rfd-model/src/schema.rs b/rfd-model/src/schema.rs index c7510e2b..bed9f4de 100644 --- a/rfd-model/src/schema.rs +++ b/rfd-model/src/schema.rs @@ -93,7 +93,7 @@ diesel::table! { github_user_id -> Integer, github_user_node_id -> Varchar, github_user_username -> Nullable, - github_user_avatar_url -> Varchar, + github_user_avatar_url -> Nullable, github_user_type -> Varchar, created_at -> Timestamptz, updated_at -> Timestamptz, diff --git a/rfd-model/src/storage/mock.rs b/rfd-model/src/storage/mock.rs index bbc48e39..84258081 100644 --- a/rfd-model/src/storage/mock.rs +++ b/rfd-model/src/storage/mock.rs @@ -258,6 +258,10 @@ impl RfdCommentUserStore for MockStorage { #[async_trait] impl RfdCommentStore for MockStorage { + async fn get(&self, id: TypedUuid) -> Result, StoreError> { + self.rfd_comment_store.as_ref().unwrap().get(id).await + } + async fn list( &self, filters: Vec, @@ -278,7 +282,7 @@ impl RfdCommentStore for MockStorage { .await } - async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { + async fn delete(&self, id: &TypedUuid) -> Result<(), StoreError> { self.rfd_comment_store.as_ref().unwrap().delete(id).await } } diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index ac568389..ddb180d8 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -316,12 +316,18 @@ pub trait RfdCommentUserStore { #[derive(Debug, Default)] pub struct RfdCommentFilter { + pub id: Option>>, pub rfd: Option>>, pub user: Option>>, pub comment_created_before: Option>, } impl RfdCommentFilter { + pub fn id(mut self, id: Option>>) -> Self { + self.id = id; + self + } + pub fn rfd(mut self, rfd: Option>>) -> Self { self.rfd = rfd; self @@ -341,11 +347,12 @@ impl RfdCommentFilter { #[cfg_attr(feature = "mock", automock)] #[async_trait] pub trait RfdCommentStore { + async fn get(&self, id: TypedUuid) -> Result, StoreError>; async fn list( &self, filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result; - async fn delete(&self, id: &TypedUuid) -> Result, StoreError>; + async fn delete(&self, id: &TypedUuid) -> Result<(), StoreError>; } diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 5f984d94..225eb74f 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -12,15 +12,19 @@ use diesel::{ sql_types::Bool, update, upsert::{excluded, on_constraint}, - BoolExpressionMethods, BoxableExpression, ExpressionMethods, SelectableHelper, + BoolExpressionMethods, BoxableExpression, ExpressionMethods, NullableExpressionMethods, + SelectableHelper, }; use newtype_uuid::{GenericUuid, TypedUuid}; use uuid::Uuid; use v_model::storage::postgres::PostgresStore; use crate::{ - db::{JobModel, RfdModel, RfdPdfModel, RfdRevisionMetaModel, RfdRevisionModel}, - schema::{job, rfd, rfd_pdf, rfd_revision}, + db::{ + JobModel, RfdCommentModel, RfdCommentUserModel, RfdModel, RfdPdfModel, + RfdRevisionMetaModel, RfdRevisionModel, + }, + schema::{job, rfd, rfd_comment, rfd_comment_user, rfd_pdf, rfd_revision}, schema_ext::Visibility, storage::StoreError, Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdRevision, Rfd, @@ -756,26 +760,144 @@ impl RfdCommentUserStore for PostgresStore { &self, new_rfd_comment_user: NewRfdCommentUser, ) -> Result { - unimplemented!() + let user: RfdCommentUserModel = insert_into(rfd_comment_user::table) + .values(( + rfd_comment_user::id.eq(new_rfd_comment_user.id.into_untyped_uuid()), + rfd_comment_user::github_user_id.eq(new_rfd_comment_user.github_user_id), + rfd_comment_user::github_user_node_id.eq(new_rfd_comment_user.github_user_node_id), + rfd_comment_user::github_user_username + .eq(new_rfd_comment_user.github_user_username), + rfd_comment_user::github_user_avatar_url + .eq(new_rfd_comment_user.github_user_avatar_url), + rfd_comment_user::github_user_type.eq(new_rfd_comment_user.github_user_type), + )) + // TODO: Handle updates + .get_result_async(&*self.pool.get().await?) + .await?; + + Ok(user.into()) } } #[async_trait] impl RfdCommentStore for PostgresStore { + async fn get(&self, id: TypedUuid) -> Result, StoreError> { + let comment = RfdCommentStore::list( + self, + vec![RfdCommentFilter::default().id(Some(vec![id]))], + &ListPagination::default().limit(1), + ) + .await?; + Ok(comment.into_iter().nth(0)) + } + async fn list( &self, filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { - unimplemented!() + let mut query = rfd_comment::table + .inner_join(rfd_comment_user::table) + .into_boxed(); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdCommentFilter { + id, + rfd, + user, + comment_created_before, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_comment::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(rfd) = rfd { + predicates.push(Box::new( + rfd_comment::rfd_id + .eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(user) = user { + predicates.push(Box::new( + rfd_comment::comment_user_id + .eq_any(user.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(comment_created_before) = comment_created_before { + predicates.push(Box::new( + rfd_comment::comment_created_at + .assume_not_null() + .le(comment_created_before), + )); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); + } + + let results = query + .offset(pagination.offset) + .limit(pagination.limit) + .get_results_async::<(RfdCommentModel, RfdCommentUserModel)>(&*self.pool.get().await?) + .await?; + + Ok(results.into_iter().map(|record| record.into()).collect()) } async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result { - unimplemented!() + let comment: RfdCommentModel = insert_into(rfd_comment::table) + .values(( + rfd_comment::id.eq(new_rfd_comment.id.into_untyped_uuid()), + rfd_comment::rfd_id.eq(new_rfd_comment.rfd_id.into_untyped_uuid()), + rfd_comment::comment_user_id.eq(new_rfd_comment.comment_user.into_untyped_uuid()), + rfd_comment::external_id.eq(new_rfd_comment.external_id), + rfd_comment::node_id.eq(new_rfd_comment.node_id), + rfd_comment::discussion_number.eq(new_rfd_comment.discussion_number), + rfd_comment::diff_hunk.eq(new_rfd_comment.diff_hunk), + rfd_comment::path.eq(new_rfd_comment.path), + rfd_comment::body.eq(new_rfd_comment.body), + rfd_comment::commit_id.eq(new_rfd_comment.commit_id), + rfd_comment::original_commit_id.eq(new_rfd_comment.original_commit_id), + rfd_comment::line.eq(new_rfd_comment.line), + rfd_comment::original_line.eq(new_rfd_comment.original_line), + rfd_comment::start_line.eq(new_rfd_comment.start_line), + rfd_comment::original_start_line.eq(new_rfd_comment.original_start_line), + rfd_comment::side.eq(new_rfd_comment.side), + rfd_comment::start_side.eq(new_rfd_comment.start_side), + rfd_comment::subject.eq(new_rfd_comment.subject), + rfd_comment::in_reply_to.eq(new_rfd_comment.in_reply_to), + rfd_comment::comment_created_at.eq(new_rfd_comment.comment_created_at), + rfd_comment::comment_updated_at.eq(new_rfd_comment.comment_updated_at), + )) + // TODO: Handle updates + .get_result_async(&*self.pool.get().await?) + .await?; + + Ok( + RfdCommentStore::get(self, TypedUuid::from_untyped_uuid(comment.id)) + .await? + .expect("Upserted comment must exist"), + ) } - async fn delete(&self, id: &TypedUuid) -> Result, StoreError> { - unimplemented!() + async fn delete(&self, id: &TypedUuid) -> Result<(), StoreError> { + let _ = update(rfd_comment::table) + .filter(rfd_comment::id.eq(id.into_untyped_uuid())) + .set(rfd_comment::deleted_at.eq(Utc::now())) + .execute_async(&*self.pool.get().await?) + .await?; + Ok(()) } } From cd132e4dbee56c3eada26e4476a4de1369c943d5 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sat, 21 Dec 2024 18:14:51 -0600 Subject: [PATCH 12/17] Handle updates on external id --- rfd-model/src/storage/postgres.rs | 35 +++++++++++++++++++++++++++++-- 1 file changed, 33 insertions(+), 2 deletions(-) diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 225eb74f..9876d152 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -771,7 +771,18 @@ impl RfdCommentUserStore for PostgresStore { .eq(new_rfd_comment_user.github_user_avatar_url), rfd_comment_user::github_user_type.eq(new_rfd_comment_user.github_user_type), )) - // TODO: Handle updates + .on_conflict(rfd_comment_user::github_user_id) + .do_update() + .set(( + rfd_comment_user::github_user_id.eq(excluded(rfd_comment_user::github_user_id)), + rfd_comment_user::github_user_node_id + .eq(excluded(rfd_comment_user::github_user_node_id)), + rfd_comment_user::github_user_username + .eq(excluded(rfd_comment_user::github_user_username)), + rfd_comment_user::github_user_avatar_url + .eq(excluded(rfd_comment_user::github_user_avatar_url)), + rfd_comment_user::github_user_type.eq(excluded(rfd_comment_user::github_user_type)), + )) .get_result_async(&*self.pool.get().await?) .await?; @@ -880,7 +891,27 @@ impl RfdCommentStore for PostgresStore { rfd_comment::comment_created_at.eq(new_rfd_comment.comment_created_at), rfd_comment::comment_updated_at.eq(new_rfd_comment.comment_updated_at), )) - // TODO: Handle updates + .on_conflict(rfd_comment::external_id) + .do_update() + .set(( + rfd_comment::comment_user_id.eq(excluded(rfd_comment::comment_user_id)), + rfd_comment::external_id.eq(excluded(rfd_comment::external_id)), + rfd_comment::node_id.eq(excluded(rfd_comment::node_id)), + rfd_comment::discussion_number.eq(excluded(rfd_comment::discussion_number)), + rfd_comment::diff_hunk.eq(excluded(rfd_comment::diff_hunk)), + rfd_comment::path.eq(excluded(rfd_comment::path)), + rfd_comment::body.eq(excluded(rfd_comment::body)), + rfd_comment::commit_id.eq(excluded(rfd_comment::commit_id)), + rfd_comment::original_commit_id.eq(excluded(rfd_comment::original_commit_id)), + rfd_comment::line.eq(excluded(rfd_comment::line)), + rfd_comment::original_line.eq(excluded(rfd_comment::original_line)), + rfd_comment::start_line.eq(excluded(rfd_comment::start_line)), + rfd_comment::original_start_line.eq(excluded(rfd_comment::original_start_line)), + rfd_comment::side.eq(excluded(rfd_comment::side)), + rfd_comment::start_side.eq(excluded(rfd_comment::start_side)), + rfd_comment::subject.eq(excluded(rfd_comment::subject)), + rfd_comment::in_reply_to.eq(excluded(rfd_comment::in_reply_to)), + )) .get_result_async(&*self.pool.get().await?) .await?; From d71c7575ef6ef98f1c8a43a535659145d3c0e038 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Sun, 22 Dec 2024 11:22:35 -0600 Subject: [PATCH 13/17] Expand model --- rfd-api-spec.json | 1033 +++++++++++++---- .../2024-12-21-204322_add_rfd_comments/up.sql | 77 +- rfd-model/src/db.rs | 64 +- rfd-model/src/lib.rs | 177 ++- rfd-model/src/schema.rs | 75 +- 5 files changed, 1135 insertions(+), 291 deletions(-) diff --git a/rfd-api-spec.json b/rfd-api-spec.json index eeeae9f1..a5cfc0ee 100644 --- a/rfd-api-spec.json +++ b/rfd-api-spec.json @@ -1395,41 +1395,6 @@ } } }, - "/meta/rfd/{number}": { - "get": { - "summary": "Get the latest representation of a RFD", - "operationId": "get_rfd_meta", - "parameters": [ - { - "in": "path", - "name": "number", - "description": "The RFD number (examples: 1 or 123)", - "required": true, - "schema": { - "type": "string" - } - } - ], - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/RfdMeta" - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - } - } - }, "/oauth/client": { "get": { "summary": "List OAuth clients", @@ -1679,17 +1644,17 @@ "/rfd": { "get": { "summary": "List all available RFDs", - "operationId": "get_rfds", + "operationId": "list_rfds", "responses": { "200": { "description": "successful operation", "content": { "application/json": { "schema": { - "title": "Array_of_RfdMeta", + "title": "Array_of_RfdWithoutContent", "type": "array", "items": { - "$ref": "#/components/schemas/RfdMeta" + "$ref": "#/components/schemas/RfdWithoutContent" } } } @@ -1738,8 +1703,8 @@ }, "/rfd/{number}": { "get": { - "summary": "Get the latest representation of a RFD", - "operationId": "get_rfd", + "summary": "Get the latest representation of an RFD's metadata", + "operationId": "view_rfd_meta", "parameters": [ { "in": "path", @@ -1757,7 +1722,51 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/FullRfd" + "$ref": "#/components/schemas/RfdWithoutContent" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/rfd/{number}/attr/{attr}": { + "get": { + "summary": "Get the an attribute of the latest revision of a RFD", + "operationId": "view_rfd_attr", + "parameters": [ + { + "in": "path", + "name": "attr", + "description": "An attribute that can be defined in an RFD document", + "required": true, + "schema": { + "$ref": "#/components/schemas/RfdAttrName" + } + }, + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdAttr" } } } @@ -1771,9 +1780,18 @@ } }, "post": { - "summary": "Replace the full document of a RFD", - "operationId": "set_rfd_document", + "summary": "Set an attribute of a RFD", + "operationId": "set_rfd_attr", "parameters": [ + { + "in": "path", + "name": "attr", + "description": "An attribute that can be defined in an RFD document", + "required": true, + "schema": { + "$ref": "#/components/schemas/RfdAttrName" + } + }, { "in": "path", "name": "number", @@ -1788,7 +1806,91 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RfdUpdateBody" + "$ref": "#/components/schemas/RfdAttrValue" + } + } + }, + "required": true + }, + "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}/comments": { + "get": { + "summary": "Get the comments related to the latest revision of a RFD", + "operationId": "view_rfd_comments", + "parameters": [ + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "title": "Array_of_RfdComment", + "type": "array", + "items": { + "$ref": "#/components/schemas/RfdComment" + } + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/rfd/{number}/content": { + "post": { + "summary": "Replace the contents of a RFD", + "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" } } }, @@ -1818,22 +1920,50 @@ } } }, - "/rfd/{number}/attr/{attr}": { - "get": { - "summary": "Get an attribute of a RFD", - "operationId": "get_rfd_attr", + "/rfd/{number}/discuss": { + "post": { + "summary": "Open a RFD for discussion", + "operationId": "discuss_rfd", "parameters": [ { "in": "path", - "name": "attr", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", "required": true, "schema": { - "$ref": "#/components/schemas/RfdAttrName" + "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}/pdf": { + "get": { + "summary": "Get the PDF locations of the latest revision of a RFD", + "operationId": "view_rfd_pdf", + "parameters": [ { "in": "path", "name": "number", + "description": "The RFD number (examples: 1 or 123)", "required": true, "schema": { "type": "string" @@ -1846,7 +1976,11 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RfdAttr" + "title": "Array_of_RfdPdf", + "type": "array", + "items": { + "$ref": "#/components/schemas/RfdPdf" + } } } } @@ -1858,22 +1992,85 @@ "$ref": "#/components/responses/Error" } } - }, + } + }, + "/rfd/{number}/publish": { "post": { - "summary": "Set an attribute of a RFD", - "operationId": "set_rfd_attr", + "summary": "Publish a RFD", + "operationId": "publish_rfd", "parameters": [ { "in": "path", - "name": "attr", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", "required": true, "schema": { - "$ref": "#/components/schemas/RfdAttrName" + "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}/raw": { + "get": { + "summary": "Get the raw contents of the latest revision of a RFD", + "operationId": "view_rfd", + "parameters": [ + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdWithContent" + } + } } }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + }, + "post": { + "summary": "Replace the full document of a RFD", + "operationId": "set_rfd_document", + "parameters": [ { "in": "path", "name": "number", + "description": "The RFD number (examples: 1 or 123)", "required": true, "schema": { "type": "string" @@ -1884,7 +2081,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RfdAttrValue" + "$ref": "#/components/schemas/RfdUpdateBody" } } }, @@ -1896,7 +2093,11 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RfdAttr" + "title": "Null", + "type": "string", + "enum": [ + null + ] } } } @@ -1910,10 +2111,10 @@ } } }, - "/rfd/{number}/content": { - "post": { - "summary": "Replace the contents of a RFD", - "operationId": "set_rfd_content", + "/rfd/{number}/revision/{revision}": { + "get": { + "summary": "Get an RFD revision's metadata", + "operationId": "view_rfd_revision_meta", "parameters": [ { "in": "path", @@ -1923,29 +2124,125 @@ "schema": { "type": "string" } + }, + { + "in": "path", + "name": "revision", + "description": "The revision id of the RFD", + "required": true, + "schema": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" + } } ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/RfdUpdateContentBody" + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdWithoutContent" + } } } }, - "required": true - }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/rfd/{number}/revision/{revision}/attr/{attr}": { + "get": { + "summary": "Get the an attribute of a revision of a RFD", + "operationId": "view_rfd_revision_attr", + "parameters": [ + { + "in": "path", + "name": "attr", + "description": "An attribute that can be defined in an RFD document", + "required": true, + "schema": { + "$ref": "#/components/schemas/RfdAttrName" + } + }, + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "revision", + "description": "The revision id of the RFD", + "required": true, + "schema": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/RfdAttr" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/rfd/{number}/revision/{revision}/comments": { + "get": { + "summary": "Get the comments related to a revision of a RFD", + "operationId": "view_rfd_revision_comments", + "parameters": [ + { + "in": "path", + "name": "number", + "description": "The RFD number (examples: 1 or 123)", + "required": true, + "schema": { + "type": "string" + } + }, + { + "in": "path", + "name": "revision", + "description": "The revision id of the RFD", + "required": true, + "schema": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" + } + } + ], "responses": { - "202": { - "description": "successfully enqueued operation", + "200": { + "description": "successful operation", "content": { "application/json": { "schema": { - "title": "Null", - "type": "string", - "enum": [ - null - ] + "title": "Array_of_RfdComment", + "type": "array", + "items": { + "$ref": "#/components/schemas/RfdComment" + } } } } @@ -1959,10 +2256,10 @@ } } }, - "/rfd/{number}/discuss": { - "post": { - "summary": "Open a RFD for discussion", - "operationId": "discuss_rfd", + "/rfd/{number}/revision/{revision}/pdf": { + "get": { + "summary": "Get the PDF locations of a revision of a RFD", + "operationId": "view_rfd_revision_pdf", "parameters": [ { "in": "path", @@ -1972,15 +2269,28 @@ "schema": { "type": "string" } + }, + { + "in": "path", + "name": "revision", + "description": "The revision id of the RFD", + "required": true, + "schema": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" + } } ], "responses": { - "202": { - "description": "successfully enqueued operation", + "200": { + "description": "successful operation", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RfdAttr" + "title": "Array_of_RfdPdf", + "type": "array", + "items": { + "$ref": "#/components/schemas/RfdPdf" + } } } } @@ -1994,10 +2304,10 @@ } } }, - "/rfd/{number}/publish": { - "post": { - "summary": "Publish a RFD", - "operationId": "publish_rfd", + "/rfd/{number}/revision/{revision}/raw": { + "get": { + "summary": "Get the raw contents of a revision of a RFD", + "operationId": "view_rfd_revision", "parameters": [ { "in": "path", @@ -2007,15 +2317,24 @@ "schema": { "type": "string" } + }, + { + "in": "path", + "name": "revision", + "description": "The revision id of the RFD", + "required": true, + "schema": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" + } } ], "responses": { - "202": { - "description": "successfully enqueued operation", + "200": { + "description": "successful operation", "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/RfdAttr" + "$ref": "#/components/schemas/RfdWithContent" } } } @@ -2611,93 +2930,6 @@ "rfd_number" ] }, - "FullRfd": { - "type": "object", - "properties": { - "authors": { - "nullable": true, - "type": "string" - }, - "commit": { - "$ref": "#/components/schemas/CommitSha" - }, - "committed_at": { - "type": "string", - "format": "date-time" - }, - "content": { - "type": "string" - }, - "discussion": { - "nullable": true, - "type": "string" - }, - "format": { - "$ref": "#/components/schemas/ContentFormat" - }, - "id": { - "$ref": "#/components/schemas/TypedUuidForRfdId" - }, - "labels": { - "nullable": true, - "type": "string" - }, - "link": { - "nullable": true, - "type": "string" - }, - "pdfs": { - "type": "array", - "items": { - "$ref": "#/components/schemas/FullRfdPdfEntry" - } - }, - "rfd_number": { - "type": "integer", - "format": "int32" - }, - "sha": { - "$ref": "#/components/schemas/FileSha" - }, - "state": { - "nullable": true, - "type": "string" - }, - "title": { - "type": "string" - }, - "visibility": { - "$ref": "#/components/schemas/Visibility" - } - }, - "required": [ - "commit", - "committed_at", - "content", - "format", - "id", - "pdfs", - "rfd_number", - "sha", - "title", - "visibility" - ] - }, - "FullRfdPdfEntry": { - "type": "object", - "properties": { - "link": { - "type": "string" - }, - "source": { - "type": "string" - } - }, - "required": [ - "link", - "source" - ] - }, "GetUserResponse_for_RfdPermission": { "type": "object", "properties": { @@ -3407,6 +3639,13 @@ "jwks_uri" ] }, + "PdfSource": { + "type": "string", + "enum": [ + "github", + "google" + ] + }, "Permissions_for_RfdPermission": { "type": "array", "items": { @@ -3445,6 +3684,9 @@ "Rfd": { "type": "object", "properties": { + "content": { + "$ref": "#/components/schemas/RfdRevision" + }, "created_at": { "type": "string", "format": "date-time" @@ -3474,6 +3716,7 @@ } }, "required": [ + "content", "created_at", "id", "rfd_number", @@ -3535,68 +3778,215 @@ } }, "required": [ - "value" + "value" + ] + }, + "RfdComment": { + "type": "object", + "properties": { + "body": { + "nullable": true, + "type": "string" + }, + "comment_created_at": { + "nullable": true, + "type": "string", + "format": "date-time" + }, + "comment_updated_at": { + "nullable": true, + "type": "string", + "format": "date-time" + }, + "comment_user": { + "$ref": "#/components/schemas/RfdCommentUser" + }, + "commit_id": { + "type": "string" + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "deleted_at": { + "nullable": true, + "type": "string", + "format": "date-time" + }, + "diff_hunk": { + "type": "string" + }, + "discussion_number": { + "nullable": true, + "type": "integer", + "format": "int32" + }, + "external_id": { + "type": "integer", + "format": "int32" + }, + "id": { + "$ref": "#/components/schemas/TypedUuidForRfdCommentId" + }, + "in_reply_to": { + "nullable": true, + "type": "integer", + "format": "int32" + }, + "line": { + "nullable": true, + "type": "integer", + "format": "int32" + }, + "node_id": { + "type": "string" + }, + "original_commit_id": { + "type": "string" + }, + "original_line": { + "nullable": true, + "type": "integer", + "format": "int32" + }, + "original_start_line": { + "nullable": true, + "type": "integer", + "format": "int32" + }, + "path": { + "type": "string" + }, + "rfd_id": { + "$ref": "#/components/schemas/TypedUuidForRfdId" + }, + "side": { + "nullable": true, + "type": "string" + }, + "start_line": { + "nullable": true, + "type": "integer", + "format": "int32" + }, + "start_side": { + "nullable": true, + "type": "string" + }, + "subject": { + "type": "string" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + }, + "required": [ + "comment_user", + "commit_id", + "created_at", + "diff_hunk", + "external_id", + "id", + "node_id", + "original_commit_id", + "path", + "rfd_id", + "subject", + "updated_at" + ] + }, + "RfdCommentUser": { + "type": "object", + "properties": { + "created_at": { + "type": "string", + "format": "date-time" + }, + "deleted_at": { + "nullable": true, + "type": "string", + "format": "date-time" + }, + "github_user_avatar_url": { + "nullable": true, + "type": "string" + }, + "github_user_id": { + "type": "integer", + "format": "int32" + }, + "github_user_node_id": { + "type": "string" + }, + "github_user_type": { + "type": "string" + }, + "github_user_username": { + "nullable": true, + "type": "string" + }, + "id": { + "$ref": "#/components/schemas/TypedUuidForRfdCommentId" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + }, + "required": [ + "created_at", + "github_user_id", + "github_user_node_id", + "github_user_type", + "id", + "updated_at" ] }, - "RfdMeta": { + "RfdPdf": { "type": "object", "properties": { - "authors": { - "nullable": true, - "type": "string" - }, - "commit": { - "$ref": "#/components/schemas/CommitSha" - }, - "committed_at": { + "created_at": { "type": "string", "format": "date-time" }, - "discussion": { + "deleted_at": { "nullable": true, - "type": "string" + "type": "string", + "format": "date-time" }, - "format": { - "$ref": "#/components/schemas/ContentFormat" + "external_id": { + "type": "string" }, "id": { - "$ref": "#/components/schemas/TypedUuidForRfdId" - }, - "labels": { - "nullable": true, - "type": "string" + "$ref": "#/components/schemas/TypedUuidForRfdPdfId" }, "link": { - "nullable": true, "type": "string" }, - "rfd_number": { - "type": "integer", - "format": "int32" - }, - "sha": { - "$ref": "#/components/schemas/FileSha" + "rfd_id": { + "$ref": "#/components/schemas/TypedUuidForRfdId" }, - "state": { - "nullable": true, - "type": "string" + "rfd_revision_id": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" }, - "title": { - "type": "string" + "source": { + "$ref": "#/components/schemas/PdfSource" }, - "visibility": { - "$ref": "#/components/schemas/Visibility" + "updated_at": { + "type": "string", + "format": "date-time" } }, "required": [ - "commit", - "committed_at", - "format", + "created_at", + "external_id", "id", - "rfd_number", - "sha", - "title", - "visibility" + "link", + "rfd_id", + "rfd_revision_id", + "source", + "updated_at" ] }, "RfdPermission": { @@ -4116,6 +4506,77 @@ } ] }, + "RfdRevision": { + "type": "object", + "properties": { + "authors": { + "nullable": true, + "type": "string" + }, + "commit": { + "$ref": "#/components/schemas/CommitSha" + }, + "committed_at": { + "type": "string", + "format": "date-time" + }, + "content": { + "type": "string" + }, + "content_format": { + "$ref": "#/components/schemas/ContentFormat" + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "deleted_at": { + "nullable": true, + "type": "string", + "format": "date-time" + }, + "discussion": { + "nullable": true, + "type": "string" + }, + "id": { + "$ref": "#/components/schemas/TypedUuidForRfdRevisionId" + }, + "labels": { + "nullable": true, + "type": "string" + }, + "rfd_id": { + "$ref": "#/components/schemas/TypedUuidForRfdId" + }, + "sha": { + "$ref": "#/components/schemas/FileSha" + }, + "state": { + "nullable": true, + "type": "string" + }, + "title": { + "type": "string" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + }, + "required": [ + "commit", + "committed_at", + "content", + "content_format", + "created_at", + "id", + "rfd_id", + "sha", + "title", + "updated_at" + ] + }, "RfdState": { "type": "string", "enum": [ @@ -4172,6 +4633,132 @@ "visibility" ] }, + "RfdWithContent": { + "type": "object", + "properties": { + "authors": { + "nullable": true, + "type": "string" + }, + "commit": { + "$ref": "#/components/schemas/CommitSha" + }, + "committed_at": { + "type": "string", + "format": "date-time" + }, + "content": { + "type": "string" + }, + "discussion": { + "nullable": true, + "type": "string" + }, + "format": { + "$ref": "#/components/schemas/ContentFormat" + }, + "id": { + "$ref": "#/components/schemas/TypedUuidForRfdId" + }, + "labels": { + "nullable": true, + "type": "string" + }, + "link": { + "nullable": true, + "type": "string" + }, + "rfd_number": { + "type": "integer", + "format": "int32" + }, + "sha": { + "$ref": "#/components/schemas/FileSha" + }, + "state": { + "nullable": true, + "type": "string" + }, + "title": { + "type": "string" + }, + "visibility": { + "$ref": "#/components/schemas/Visibility" + } + }, + "required": [ + "commit", + "committed_at", + "content", + "format", + "id", + "rfd_number", + "sha", + "title", + "visibility" + ] + }, + "RfdWithoutContent": { + "type": "object", + "properties": { + "authors": { + "nullable": true, + "type": "string" + }, + "commit": { + "$ref": "#/components/schemas/CommitSha" + }, + "committed_at": { + "type": "string", + "format": "date-time" + }, + "discussion": { + "nullable": true, + "type": "string" + }, + "format": { + "$ref": "#/components/schemas/ContentFormat" + }, + "id": { + "$ref": "#/components/schemas/TypedUuidForRfdId" + }, + "labels": { + "nullable": true, + "type": "string" + }, + "link": { + "nullable": true, + "type": "string" + }, + "rfd_number": { + "type": "integer", + "format": "int32" + }, + "sha": { + "$ref": "#/components/schemas/FileSha" + }, + "state": { + "nullable": true, + "type": "string" + }, + "title": { + "type": "string" + }, + "visibility": { + "$ref": "#/components/schemas/Visibility" + } + }, + "required": [ + "commit", + "committed_at", + "format", + "id", + "rfd_number", + "sha", + "title", + "visibility" + ] + }, "SearchResultHit": { "type": "object", "properties": { @@ -4302,10 +4889,22 @@ "type": "string", "format": "uuid" }, + "TypedUuidForRfdCommentId": { + "type": "string", + "format": "uuid" + }, "TypedUuidForRfdId": { "type": "string", "format": "uuid" }, + "TypedUuidForRfdPdfId": { + "type": "string", + "format": "uuid" + }, + "TypedUuidForRfdRevisionId": { + "type": "string", + "format": "uuid" + }, "TypedUuidForUserId": { "type": "string", "format": "uuid" diff --git a/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql b/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql index cb521f19..4c334ba5 100644 --- a/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql +++ b/rfd-model/migrations/2024-12-21-204322_add_rfd_comments/up.sql @@ -1,28 +1,53 @@ CREATE TABLE rfd_comment_user ( id UUID PRIMARY KEY, - github_user_id INTEGER NOT NULL, - github_user_node_id VARCHAR NOT NULL, - github_user_username VARCHAR, - github_user_avatar_url VARCHAR, - github_user_type VARCHAR NOT NULL, + external_id INTEGER NOT NULL, + node_id VARCHAR NOT NULL, + + user_username VARCHAR, + user_avatar_url VARCHAR, + user_type VARCHAR NOT NULL, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), deleted_at TIMESTAMPTZ, - UNIQUE(github_user_id), - UNIQUE(github_user_node_id) + UNIQUE(external_id), + UNIQUE(node_id) ); -CREATE TABLE rfd_comment ( +CREATE TABLE rfd_review ( + id UUID PRIMARY KEY, + rfd_id UUID REFERENCES rfd (id) NOT NULL, + comment_user_id UUID REFERENCES rfd_comment_user(id) NOT NULL, + external_id INTEGER NOT NULL, + node_id VARCHAR NOT NULL, + + body VARCHAR NOT NULL, + state VARCHAR NOT NULL, + commit_id VARCHAR NOT NULL, + + review_created_at TIMESTAMPTZ NOT NULL, + + created_at TIMESTAMPTZ NOT NULL, + updated_at TIMESTAMPTZ NOT NULL, + deleted_at TIMESTAMPTZ, + + UNIQUE(external_id), + UNIQUE(node_id) +); + +CREATE TABLE rfd_review_comment ( id UUID PRIMARY KEY, rfd_id UUID REFERENCES rfd (id) NOT NULL, comment_user_id UUID REFERENCES rfd_comment_user(id) NOT NULL, external_id INTEGER NOT NULL, node_id -> VARCHAR NOT NULL, - discussion_number INTEGER, + + review_id UUID REFERENCES rfd_review(id), + diff_hunk -> VARCHAR NOT NULL, path -> VARCHAR NOT NULL, - body -> VARCHAR, + body -> VARCHAR NOT NULL, commit_id -> VARCHAR NOT NULL, original_commit_id -> VARCHAR NOT NULL, line INTEGER, @@ -33,11 +58,33 @@ CREATE TABLE rfd_comment ( start_side -> VARCHAR, subject VARCHAR NOT NULL, in_reply_to INTEGER, - comment_created_at TIMESTAMPTZ, - comment_updated_at TIMESTAMPTZ, - created_at -> TIMESTAMPTZ NOT NULL, - updated_at -> TIMESTAMPTZ NOT NULL, - deleted_at -> TIMESTAMPTZ, + + comment_created_at TIMESTAMPTZ NOT NULL, + comment_updated_at TIMESTAMPTZ NOT NULL, + + created_at TIMESTAMPTZ NOT NULL, + updated_at TIMESTAMPTZ NOT NULL, + deleted_at TIMESTAMPTZ, + + UNIQUE(external_id), + UNIQUE(node_id) +); + +CREATE TABLE rfd_comment ( + id UUID PRIMARY KEY, + rfd_id UUID REFERENCES rfd (id) NOT NULL, + comment_user_id UUID REFERENCES rfd_comment_user(id) NOT NULL, + external_id INTEGER NOT NULL, + node_id VARCHAR NOT NULL, + + body VARCHAR, + + comment_created_at TIMESTAMPTZ NOT NULL, + comment_updated_at TIMESTAMPTZ NOT NULL, + + created_at TIMESTAMPTZ NOT NULL, + updated_at TIMESTAMPTZ NOT NULL, + deleted_at TIMESTAMPTZ, UNIQUE(external_id), UNIQUE(node_id) diff --git a/rfd-model/src/db.rs b/rfd-model/src/db.rs index 2a09bf5e..8fd647e7 100644 --- a/rfd-model/src/db.rs +++ b/rfd-model/src/db.rs @@ -9,7 +9,10 @@ use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::{ - schema::{job, rfd, rfd_comment, rfd_comment_user, rfd_pdf, rfd_revision}, + schema::{ + job, rfd, rfd_comment, rfd_comment_user, rfd_pdf, rfd_review, rfd_review_comment, + rfd_revision, + }, schema_ext::{ContentFormat, PdfSource, Visibility}, }; @@ -77,32 +80,49 @@ pub struct JobModel { pub started_at: Option>, } -#[derive(Debug, Deserialize, Serialize, Queryable, Insertable)] +#[derive(Clone, Debug, Deserialize, Serialize, Queryable, Insertable)] #[diesel(table_name = rfd_comment_user)] pub struct RfdCommentUserModel { pub id: Uuid, - pub github_user_id: i32, - pub github_user_node_id: String, - pub github_user_username: Option, - pub github_user_avatar_url: Option, - pub github_user_type: String, + pub external_id: i32, + pub node_id: String, + pub user_username: Option, + pub user_avatar_url: Option, + pub user_type: String, pub created_at: DateTime, pub updated_at: DateTime, pub deleted_at: Option>, } -#[derive(Debug, Deserialize, Serialize, Queryable, Insertable)] -#[diesel(table_name = rfd_comment)] -pub struct RfdCommentModel { +#[derive(Clone, Debug, Deserialize, Serialize, Queryable, Insertable)] +#[diesel(table_name = rfd_review)] +pub struct RfdReviewModel { pub id: Uuid, pub rfd_id: Uuid, pub comment_user_id: Uuid, pub external_id: i32, pub node_id: String, - pub discussion_number: Option, + pub body: String, + pub state: String, + pub commit_id: String, + pub review_created_at: DateTime, + pub created_at: DateTime, + pub updated_at: DateTime, + pub deleted_at: Option>, +} + +#[derive(Clone, Debug, Deserialize, Serialize, Queryable, Insertable)] +#[diesel(table_name = rfd_review_comment)] +pub struct RfdReviewCommentModel { + pub id: Uuid, + pub rfd_id: Uuid, + pub comment_user_id: Uuid, + pub external_id: i32, + pub node_id: String, + pub review_id: Option, pub diff_hunk: String, pub path: String, - pub body: Option, + pub body: String, pub commit_id: String, pub original_commit_id: String, pub line: Option, @@ -113,8 +133,24 @@ pub struct RfdCommentModel { pub start_side: Option, pub subject: String, pub in_reply_to: Option, - pub comment_created_at: Option>, - pub comment_updated_at: Option>, + pub comment_created_at: DateTime, + pub comment_updated_at: DateTime, + pub created_at: DateTime, + pub updated_at: DateTime, + pub deleted_at: Option>, +} + +#[derive(Clone, Debug, Deserialize, Serialize, Queryable, Insertable)] +#[diesel(table_name = rfd_comment)] +pub struct RfdCommentModel { + pub id: Uuid, + pub rfd_id: Uuid, + pub comment_user_id: Uuid, + pub external_id: i32, + pub node_id: String, + pub body: String, + pub comment_created_at: DateTime, + pub comment_updated_at: DateTime, pub created_at: DateTime, pub updated_at: DateTime, pub deleted_at: Option>, diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index 9ce2efbf..1ae3afb0 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -4,8 +4,8 @@ use chrono::{DateTime, Utc}; use db::{ - JobModel, RfdCommentModel, RfdCommentUserModel, RfdModel, RfdPdfModel, RfdRevisionMetaModel, - RfdRevisionModel, + JobModel, RfdCommentModel, RfdCommentUserModel, RfdModel, RfdPdfModel, RfdReviewCommentModel, + RfdReviewModel, RfdRevisionMetaModel, RfdRevisionModel, }; use newtype_uuid::{GenericUuid, TypedUuid, TypedUuidKind, TypedUuidTag}; use partial_struct::partial; @@ -315,17 +315,17 @@ impl TypedUuidKind for RfdCommentUserId { #[partial(NewRfdCommentUser)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] pub struct RfdCommentUser { - pub id: TypedUuid, - pub github_user_id: i32, - pub github_user_node_id: String, - pub github_user_username: Option, - pub github_user_avatar_url: Option, - pub github_user_type: String, - #[partial(NewRfdComment(skip))] + pub id: TypedUuid, + pub external_id: i32, + pub node_id: String, + pub user_username: Option, + pub user_avatar_url: Option, + pub user_type: String, + #[partial(NewRfdCommentUser(skip))] pub created_at: DateTime, - #[partial(NewRfdComment(skip))] + #[partial(NewRfdCommentUser(skip))] pub updated_at: DateTime, - #[partial(NewRfdComment(skip))] + #[partial(NewRfdCommentUser(skip))] pub deleted_at: Option>, } @@ -333,11 +333,11 @@ impl From for RfdCommentUser { fn from(value: RfdCommentUserModel) -> Self { Self { id: TypedUuid::from_untyped_uuid(value.id), - github_user_id: value.github_user_id, - github_user_node_id: value.github_user_node_id, - github_user_username: value.github_user_username, - github_user_avatar_url: value.github_user_avatar_url, - github_user_type: value.github_user_type, + external_id: value.external_id, + node_id: value.node_id, + user_username: value.user_username, + user_avatar_url: value.user_avatar_url, + user_type: value.user_type, created_at: value.created_at, updated_at: value.updated_at, deleted_at: value.deleted_at, @@ -346,27 +346,77 @@ impl From for RfdCommentUser { } #[derive(JsonSchema)] -pub enum RfdCommentId {} -impl TypedUuidKind for RfdCommentId { +pub enum RfdReviewId {} +impl TypedUuidKind for RfdReviewId { fn tag() -> TypedUuidTag { - const TAG: TypedUuidTag = TypedUuidTag::new("rfd-comment"); + const TAG: TypedUuidTag = TypedUuidTag::new("rfd-review"); TAG } } -#[partial(NewRfdComment)] +#[partial(NewRfdReview)] #[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] -pub struct RfdComment { - pub id: TypedUuid, +pub struct RfdReview { + pub id: TypedUuid, pub rfd_id: TypedUuid, - #[partial(NewRfdComment(retype = TypedUuid))] + #[partial(NewRfdReview(retype = TypedUuid))] pub comment_user: RfdCommentUser, pub external_id: i32, pub node_id: String, - pub discussion_number: Option, + pub body: String, + pub state: String, + pub commit_id: String, + pub review_created_at: DateTime, + #[partial(NewRfdReview(skip))] + pub created_at: DateTime, + #[partial(NewRfdReview(skip))] + pub updated_at: DateTime, + #[partial(NewRfdReview(skip))] + pub deleted_at: Option>, +} + +impl From<(RfdReviewModel, RfdCommentUserModel)> for RfdReview { + fn from((review, user): (RfdReviewModel, RfdCommentUserModel)) -> Self { + Self { + id: TypedUuid::from_untyped_uuid(review.id), + rfd_id: TypedUuid::from_untyped_uuid(review.rfd_id), + comment_user: user.into(), + external_id: review.external_id, + node_id: review.node_id, + body: review.body, + state: review.state, + commit_id: review.commit_id, + review_created_at: review.review_created_at, + created_at: review.created_at, + updated_at: review.updated_at, + deleted_at: review.deleted_at, + } + } +} + +#[derive(JsonSchema)] +pub enum RfdReviewCommentId {} +impl TypedUuidKind for RfdReviewCommentId { + fn tag() -> TypedUuidTag { + const TAG: TypedUuidTag = TypedUuidTag::new("rfd-review-comment"); + TAG + } +} + +#[partial(NewRfdReviewComment)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct RfdReviewComment { + pub id: TypedUuid, + pub rfd_id: TypedUuid, + #[partial(NewRfdReviewComment(retype = TypedUuid))] + pub comment_user: RfdCommentUser, + pub external_id: i32, + pub node_id: String, + #[partial(NewRfdReviewComment(retype = Option>))] + pub review: Option, pub diff_hunk: String, pub path: String, - pub body: Option, + pub body: String, pub commit_id: String, pub original_commit_id: String, pub line: Option, @@ -377,25 +427,37 @@ pub struct RfdComment { pub start_side: Option, pub subject: String, pub in_reply_to: Option, - pub comment_created_at: Option>, - pub comment_updated_at: Option>, - #[partial(NewRfdComment(skip))] + pub comment_created_at: DateTime, + pub comment_updated_at: DateTime, + #[partial(NewRfdReviewComment(skip))] pub created_at: DateTime, - #[partial(NewRfdComment(skip))] + #[partial(NewRfdReviewComment(skip))] pub updated_at: DateTime, - #[partial(NewRfdComment(skip))] + #[partial(NewRfdReviewComment(skip))] pub deleted_at: Option>, } -impl From<(RfdCommentModel, RfdCommentUserModel)> for RfdComment { - fn from((comment, user): (RfdCommentModel, RfdCommentUserModel)) -> Self { +impl + From<( + RfdReviewCommentModel, + RfdCommentUserModel, + Option, + )> for RfdReviewComment +{ + fn from( + (comment, user, review): ( + RfdReviewCommentModel, + RfdCommentUserModel, + Option, + ), + ) -> Self { Self { id: TypedUuid::from_untyped_uuid(comment.id), rfd_id: TypedUuid::from_untyped_uuid(comment.rfd_id), - comment_user: user.into(), + comment_user: user.clone().into(), external_id: comment.external_id, node_id: comment.node_id, - discussion_number: comment.discussion_number, + review: review.map(|r| (r, user).into()), diff_hunk: comment.diff_hunk, path: comment.path, body: comment.body, @@ -417,3 +479,50 @@ impl From<(RfdCommentModel, RfdCommentUserModel)> for RfdComment { } } } + +#[derive(JsonSchema)] +pub enum RfdCommentId {} +impl TypedUuidKind for RfdCommentId { + fn tag() -> TypedUuidTag { + const TAG: TypedUuidTag = TypedUuidTag::new("rfd-review-comment"); + TAG + } +} + +#[partial(NewRfdComment)] +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize, JsonSchema)] +pub struct RfdComment { + pub id: TypedUuid, + pub rfd_id: TypedUuid, + #[partial(NewRfdComment(retype = TypedUuid))] + pub comment_user: RfdCommentUser, + pub external_id: i32, + pub node_id: String, + pub body: String, + pub comment_created_at: DateTime, + pub comment_updated_at: DateTime, + #[partial(NewRfdComment(skip))] + pub created_at: DateTime, + #[partial(NewRfdComment(skip))] + pub updated_at: DateTime, + #[partial(NewRfdComment(skip))] + pub deleted_at: Option>, +} + +impl From<(RfdCommentModel, RfdCommentUserModel)> for RfdComment { + fn from((comment, user): (RfdCommentModel, RfdCommentUserModel)) -> Self { + Self { + id: TypedUuid::from_untyped_uuid(comment.id), + rfd_id: TypedUuid::from_untyped_uuid(comment.rfd_id), + comment_user: user.into(), + external_id: comment.external_id, + node_id: comment.node_id, + body: comment.body, + comment_created_at: comment.comment_created_at, + comment_updated_at: comment.comment_updated_at, + created_at: comment.created_at, + updated_at: comment.updated_at, + deleted_at: comment.deleted_at, + } + } +} diff --git a/rfd-model/src/schema.rs b/rfd-model/src/schema.rs index bed9f4de..b5ebc9cb 100644 --- a/rfd-model/src/schema.rs +++ b/rfd-model/src/schema.rs @@ -90,11 +90,13 @@ diesel::table! { diesel::table! { rfd_comment_user (id) { id -> Uuid, - github_user_id -> Integer, - github_user_node_id -> Varchar, - github_user_username -> Nullable, - github_user_avatar_url -> Nullable, - github_user_type -> Varchar, + external_id -> Integer, + node_id -> Varchar, + + user_username -> Nullable, + user_avatar_url -> Nullable, + user_type -> Varchar, + created_at -> Timestamptz, updated_at -> Timestamptz, deleted_at -> Nullable, @@ -102,16 +104,38 @@ diesel::table! { } diesel::table! { - rfd_comment (id) { + rfd_review (id) { + id -> Uuid, + rfd_id -> Uuid, + comment_user_id -> Uuid, + external_id -> Integer, + node_id -> Varchar, + + body -> Varchar, + state -> Varchar, + commit_id -> Varchar, + + review_created_at -> Timestamptz, + + created_at -> Timestamptz, + updated_at -> Timestamptz, + deleted_at -> Nullable, + } +} + +diesel::table! { + rfd_review_comment (id) { id -> Uuid, rfd_id -> Uuid, comment_user_id -> Uuid, external_id -> Integer, node_id -> Varchar, - discussion_number -> Nullable, + + review_id -> Nullable, + diff_hunk -> Varchar, path -> Varchar, - body -> Nullable, + body -> Varchar, commit_id -> Varchar, original_commit_id -> Varchar, line -> Nullable, @@ -122,8 +146,29 @@ diesel::table! { start_side -> Nullable, subject -> Varchar, in_reply_to -> Nullable, - comment_created_at -> Nullable, - comment_updated_at -> Nullable, + + comment_created_at -> Timestamptz, + comment_updated_at -> Timestamptz, + + created_at -> Timestamptz, + updated_at -> Timestamptz, + deleted_at -> Nullable, + } +} + +diesel::table! { + rfd_comment (id) { + id -> Uuid, + rfd_id -> Uuid, + comment_user_id -> Uuid, + external_id -> Integer, + node_id -> Varchar, + + body -> Varchar, + + comment_created_at -> Timestamptz, + comment_updated_at -> Timestamptz, + created_at -> Timestamptz, updated_at -> Timestamptz, deleted_at -> Nullable, @@ -133,8 +178,14 @@ diesel::table! { diesel::joinable!(rfd_pdf -> rfd (rfd_id)); diesel::joinable!(rfd_pdf -> rfd_revision (rfd_revision_id)); diesel::joinable!(rfd_revision -> rfd (rfd_id)); + diesel::joinable!(rfd_comment -> rfd (rfd_id)); diesel::joinable!(rfd_comment -> rfd_comment_user (comment_user_id)); +diesel::joinable!(rfd_review -> rfd (rfd_id)); +diesel::joinable!(rfd_review -> rfd_comment_user (comment_user_id)); +diesel::joinable!(rfd_review_comment -> rfd (rfd_id)); +diesel::joinable!(rfd_review_comment -> rfd_comment_user (comment_user_id)); +diesel::joinable!(rfd_review_comment -> rfd_review (review_id)); diesel::allow_tables_to_appear_in_same_query!( job, @@ -142,5 +193,7 @@ diesel::allow_tables_to_appear_in_same_query!( rfd_pdf, rfd_revision, rfd_comment, - rfd_comment_user + rfd_comment_user, + rfd_review, + rfd_review_comment, ); From 41a01d033fd84442311646be8e64341746e86596 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Mon, 23 Dec 2024 09:18:21 -0600 Subject: [PATCH 14/17] Redo models --- rfd-api/src/context.rs | 12 +- rfd-api/src/endpoints/rfd.rs | 8 +- rfd-model/src/lib.rs | 44 +-- rfd-model/src/storage/mock.rs | 126 ++++++++- rfd-model/src/storage/mod.rs | 139 ++++++++- rfd-model/src/storage/postgres.rs | 450 +++++++++++++++++++++++++----- 6 files changed, 653 insertions(+), 126 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index df317f60..7214c8e8 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -18,10 +18,10 @@ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, storage::{ - JobStore, RfdCommentFilter, RfdCommentStore, RfdFilter, RfdMetaStore, RfdPdfFilter, - RfdPdfStore, RfdStorage, RfdStore, + JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdReviewCommentFilter, + RfdReviewCommentStore, RfdStorage, RfdStore, }, - CommitSha, FileSha, Job, NewJob, Rfd, RfdComment, RfdId, RfdMeta, RfdPdf, RfdRevision, + CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdPdf, RfdReviewComment, RfdRevision, RfdRevisionId, }; use rsa::{ @@ -521,11 +521,11 @@ impl RfdContext { caller: &Caller, rfd_number: i32, revision: Option, - ) -> ResourceResult, StoreError> { + ) -> ResourceResult, StoreError> { let rfd = self.get_rfd_meta(caller, rfd_number, revision).await?; - let comments = RfdCommentStore::list( + let comments = RfdReviewCommentStore::list( &*self.storage, - vec![RfdCommentFilter::default() + vec![RfdReviewCommentFilter::default() .rfd(Some(vec![rfd.id])) .comment_created_before(Some(rfd.content.committed_at))], &ListPagination::unlimited(), diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 600c73b5..c68c14fc 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -13,7 +13,7 @@ use rfd_data::{ }; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - Rfd, RfdComment, RfdPdf, RfdRevisionId, + Rfd, RfdPdf, RfdReviewComment, RfdRevisionId, }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -175,7 +175,7 @@ pub async fn view_rfd_attr( pub async fn view_rfd_comments( rqctx: RequestContext, path: Path, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); @@ -269,7 +269,7 @@ pub async fn view_rfd_revision_attr( pub async fn view_rfd_revision_comments( rqctx: RequestContext, path: Path, -) -> Result>, HttpError> { +) -> Result>, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); @@ -433,7 +433,7 @@ async fn view_rfd_comments_op( caller: &Caller, number: String, revision: Option, -) -> Result>, HttpError> { +) -> Result>, HttpError> { if let Ok(rfd_number) = number.parse::() { let comments = ctx.view_rfd_comments(caller, rfd_number, revision).await?; Ok(HttpResponseOk(comments)) diff --git a/rfd-model/src/lib.rs b/rfd-model/src/lib.rs index 1ae3afb0..1ba4b571 100644 --- a/rfd-model/src/lib.rs +++ b/rfd-model/src/lib.rs @@ -359,8 +359,7 @@ impl TypedUuidKind for RfdReviewId { pub struct RfdReview { pub id: TypedUuid, pub rfd_id: TypedUuid, - #[partial(NewRfdReview(retype = TypedUuid))] - pub comment_user: RfdCommentUser, + pub comment_user_id: TypedUuid, pub external_id: i32, pub node_id: String, pub body: String, @@ -375,12 +374,12 @@ pub struct RfdReview { pub deleted_at: Option>, } -impl From<(RfdReviewModel, RfdCommentUserModel)> for RfdReview { - fn from((review, user): (RfdReviewModel, RfdCommentUserModel)) -> Self { +impl From for RfdReview { + fn from(review: RfdReviewModel) -> Self { Self { id: TypedUuid::from_untyped_uuid(review.id), rfd_id: TypedUuid::from_untyped_uuid(review.rfd_id), - comment_user: user.into(), + comment_user_id: TypedUuid::from_untyped_uuid(review.comment_user_id), external_id: review.external_id, node_id: review.node_id, body: review.body, @@ -408,12 +407,10 @@ impl TypedUuidKind for RfdReviewCommentId { pub struct RfdReviewComment { pub id: TypedUuid, pub rfd_id: TypedUuid, - #[partial(NewRfdReviewComment(retype = TypedUuid))] - pub comment_user: RfdCommentUser, + pub comment_user_id: TypedUuid, pub external_id: i32, pub node_id: String, - #[partial(NewRfdReviewComment(retype = Option>))] - pub review: Option, + pub review_id: Option>, pub diff_hunk: String, pub path: String, pub body: String, @@ -437,27 +434,15 @@ pub struct RfdReviewComment { pub deleted_at: Option>, } -impl - From<( - RfdReviewCommentModel, - RfdCommentUserModel, - Option, - )> for RfdReviewComment -{ - fn from( - (comment, user, review): ( - RfdReviewCommentModel, - RfdCommentUserModel, - Option, - ), - ) -> Self { +impl From for RfdReviewComment { + fn from(comment: RfdReviewCommentModel) -> Self { Self { id: TypedUuid::from_untyped_uuid(comment.id), rfd_id: TypedUuid::from_untyped_uuid(comment.rfd_id), - comment_user: user.clone().into(), + comment_user_id: TypedUuid::from_untyped_uuid(comment.comment_user_id), external_id: comment.external_id, node_id: comment.node_id, - review: review.map(|r| (r, user).into()), + review_id: comment.review_id.map(TypedUuid::from_untyped_uuid), diff_hunk: comment.diff_hunk, path: comment.path, body: comment.body, @@ -494,8 +479,7 @@ impl TypedUuidKind for RfdCommentId { pub struct RfdComment { pub id: TypedUuid, pub rfd_id: TypedUuid, - #[partial(NewRfdComment(retype = TypedUuid))] - pub comment_user: RfdCommentUser, + pub comment_user_id: TypedUuid, pub external_id: i32, pub node_id: String, pub body: String, @@ -509,12 +493,12 @@ pub struct RfdComment { pub deleted_at: Option>, } -impl From<(RfdCommentModel, RfdCommentUserModel)> for RfdComment { - fn from((comment, user): (RfdCommentModel, RfdCommentUserModel)) -> Self { +impl From for RfdComment { + fn from(comment: RfdCommentModel) -> Self { Self { id: TypedUuid::from_untyped_uuid(comment.id), rfd_id: TypedUuid::from_untyped_uuid(comment.rfd_id), - comment_user: user.into(), + comment_user_id: TypedUuid::from_untyped_uuid(comment.comment_user_id), external_id: comment.external_id, node_id: comment.node_id, body: comment.body, diff --git a/rfd-model/src/storage/mock.rs b/rfd-model/src/storage/mock.rs index 84258081..56a7b6ab 100644 --- a/rfd-model/src/storage/mock.rs +++ b/rfd-model/src/storage/mock.rs @@ -8,17 +8,20 @@ use std::sync::Arc; use v_model::storage::StoreError; use crate::{ - Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdRevision, Rfd, - RfdComment, RfdCommentId, RfdCommentUser, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdRevision, - RfdRevisionId, RfdRevisionMeta, + Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdReview, + NewRfdReviewComment, NewRfdRevision, Rfd, RfdComment, RfdCommentId, RfdCommentUser, + RfdCommentUserId, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdReview, RfdReviewComment, + RfdReviewCommentId, RfdReviewId, RfdRevision, RfdRevisionId, RfdRevisionMeta, }; use super::{ JobFilter, JobStore, ListPagination, MockJobStore, MockRfdCommentStore, - MockRfdCommentUserStore, MockRfdMetaStore, MockRfdPdfStore, MockRfdRevisionMetaStore, - MockRfdRevisionStore, MockRfdStore, RfdCommentFilter, RfdCommentStore, RfdCommentUserStore, - RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, - RfdRevisionStore, RfdStore, + MockRfdCommentUserStore, MockRfdMetaStore, MockRfdPdfStore, MockRfdReviewCommentStore, + MockRfdReviewStore, MockRfdRevisionMetaStore, MockRfdRevisionStore, MockRfdStore, + RfdCommentFilter, RfdCommentStore, RfdCommentUserFilter, RfdCommentUserStore, RfdFilter, + RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdReviewCommentFilter, RfdReviewCommentStore, + RfdReviewFilter, RfdReviewStore, RfdRevisionFilter, RfdRevisionMetaStore, RfdRevisionStore, + RfdStore, }; pub struct MockStorage { @@ -29,6 +32,8 @@ pub struct MockStorage { pub rfd_pdf_store: Option>, pub job_store: Option>, pub rfd_comment_user_store: Option>, + pub rfd_review_store: Option>, + pub rfd_review_comment_store: Option>, pub rfd_comment_store: Option>, } @@ -42,6 +47,8 @@ impl MockStorage { rfd_pdf_store: None, job_store: None, rfd_comment_user_store: None, + rfd_review_store: None, + rfd_review_comment_store: None, rfd_comment_store: None, } } @@ -244,6 +251,23 @@ impl JobStore for MockStorage { #[async_trait] impl RfdCommentUserStore for MockStorage { + async fn get( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + self.rfd_comment_user_store.as_ref().unwrap().get(id).await + } + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_comment_user_store + .as_ref() + .unwrap() + .list(filters, pagination) + .await + } async fn upsert( &self, new_rfd_comment_user: NewRfdCommentUser, @@ -254,6 +278,89 @@ impl RfdCommentUserStore for MockStorage { .upsert(new_rfd_comment_user) .await } + async fn delete( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + self.rfd_comment_user_store + .as_ref() + .unwrap() + .delete(id) + .await + } +} + +#[async_trait] +impl RfdReviewStore for MockStorage { + async fn get(&self, id: TypedUuid) -> Result, StoreError> { + self.rfd_review_store.as_ref().unwrap().get(id).await + } + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_review_store + .as_ref() + .unwrap() + .list(filters, pagination) + .await + } + async fn upsert(&self, new_rfd_comment: NewRfdReview) -> Result { + self.rfd_review_store + .as_ref() + .unwrap() + .upsert(new_rfd_comment) + .await + } + async fn delete(&self, id: TypedUuid) -> Result, StoreError> { + self.rfd_review_store.as_ref().unwrap().delete(id).await + } +} + +#[async_trait] +impl RfdReviewCommentStore for MockStorage { + async fn get( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + self.rfd_review_comment_store + .as_ref() + .unwrap() + .get(id) + .await + } + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + self.rfd_review_comment_store + .as_ref() + .unwrap() + .list(filters, pagination) + .await + } + async fn upsert( + &self, + new_rfd_review_comment: NewRfdReviewComment, + ) -> Result { + self.rfd_review_comment_store + .as_ref() + .unwrap() + .upsert(new_rfd_review_comment) + .await + } + async fn delete( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + self.rfd_review_comment_store + .as_ref() + .unwrap() + .delete(id) + .await + } } #[async_trait] @@ -261,7 +368,6 @@ impl RfdCommentStore for MockStorage { async fn get(&self, id: TypedUuid) -> Result, StoreError> { self.rfd_comment_store.as_ref().unwrap().get(id).await } - async fn list( &self, filters: Vec, @@ -273,7 +379,6 @@ impl RfdCommentStore for MockStorage { .list(filters, pagination) .await } - async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result { self.rfd_comment_store .as_ref() @@ -281,8 +386,7 @@ impl RfdCommentStore for MockStorage { .upsert(new_rfd_comment) .await } - - async fn delete(&self, id: &TypedUuid) -> Result<(), StoreError> { + async fn delete(&self, id: TypedUuid) -> Result, StoreError> { self.rfd_comment_store.as_ref().unwrap().delete(id).await } } diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index ddb180d8..5a6cc68a 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -14,8 +14,9 @@ use v_model::storage::{ListPagination, StoreError}; use crate::{ schema_ext::PdfSource, CommitSha, Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, - NewRfdPdf, NewRfdRevision, Rfd, RfdComment, RfdCommentId, RfdCommentUser, RfdCommentUserId, - RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdRevision, RfdRevisionId, RfdRevisionMeta, + NewRfdPdf, NewRfdReview, NewRfdReviewComment, NewRfdRevision, Rfd, RfdComment, RfdCommentId, + RfdCommentUser, RfdCommentUserId, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdReview, + RfdReviewComment, RfdReviewCommentId, RfdReviewId, RfdRevision, RfdRevisionId, RfdRevisionMeta, }; #[cfg(feature = "mock")] @@ -30,7 +31,7 @@ pub trait RfdStorage: + RfdPdfStore + JobStore + RfdCommentUserStore - + RfdCommentStore + + RfdReviewCommentStore + Send + Sync + 'static @@ -44,7 +45,7 @@ impl RfdStorage for T where + RfdPdfStore + JobStore + RfdCommentUserStore - + RfdCommentStore + + RfdReviewCommentStore + Send + Sync + 'static @@ -305,13 +306,139 @@ pub trait JobStore { async fn complete(&self, id: i32) -> Result, StoreError>; } +#[derive(Debug, Default)] +pub struct RfdCommentUserFilter { + pub id: Option>>, +} + +impl RfdCommentUserFilter { + pub fn id(mut self, id: Option>>) -> Self { + self.id = id; + self + } +} + #[cfg_attr(feature = "mock", automock)] #[async_trait] pub trait RfdCommentUserStore { + async fn get( + &self, + id: TypedUuid, + ) -> Result, StoreError>; + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError>; async fn upsert( &self, new_rfd_comment_user: NewRfdCommentUser, ) -> Result; + async fn delete( + &self, + id: TypedUuid, + ) -> Result, StoreError>; +} + +#[derive(Debug, Default)] +pub struct RfdReviewFilter { + pub id: Option>>, + pub rfd: Option>>, + pub user: Option>>, + pub review_created_before: Option>, +} + +impl RfdReviewFilter { + pub fn id(mut self, id: Option>>) -> Self { + self.id = id; + self + } + + pub fn rfd(mut self, rfd: Option>>) -> Self { + self.rfd = rfd; + self + } + + pub fn user(mut self, user: Option>>) -> Self { + self.user = user; + self + } + + pub fn review_created_before(mut self, review_created_before: Option>) -> Self { + self.review_created_before = review_created_before; + self + } +} + +#[cfg_attr(feature = "mock", automock)] +#[async_trait] +pub trait RfdReviewStore { + async fn get(&self, id: TypedUuid) -> Result, StoreError>; + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError>; + async fn upsert(&self, new_rfd_review: NewRfdReview) -> Result; + async fn delete(&self, id: TypedUuid) -> Result, StoreError>; +} + +#[derive(Debug, Default)] +pub struct RfdReviewCommentFilter { + pub id: Option>>, + pub rfd: Option>>, + pub user: Option>>, + pub review: Option>>, + pub comment_created_before: Option>, +} + +impl RfdReviewCommentFilter { + pub fn id(mut self, id: Option>>) -> Self { + self.id = id; + self + } + + pub fn rfd(mut self, rfd: Option>>) -> Self { + self.rfd = rfd; + self + } + + pub fn user(mut self, user: Option>>) -> Self { + self.user = user; + self + } + + pub fn review(mut self, review: Option>>) -> Self { + self.review = review; + self + } + + pub fn comment_created_before(mut self, comment_created_before: Option>) -> Self { + self.comment_created_before = comment_created_before; + self + } +} + +#[cfg_attr(feature = "mock", automock)] +#[async_trait] +pub trait RfdReviewCommentStore { + async fn get( + &self, + id: TypedUuid, + ) -> Result, StoreError>; + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError>; + async fn upsert( + &self, + new_rfd_review_comment: NewRfdReviewComment, + ) -> Result; + async fn delete( + &self, + id: TypedUuid, + ) -> Result, StoreError>; } #[derive(Debug, Default)] @@ -353,6 +480,6 @@ pub trait RfdCommentStore { filters: Vec, pagination: &ListPagination, ) -> Result, StoreError>; - async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result; - async fn delete(&self, id: &TypedUuid) -> Result<(), StoreError>; + async fn upsert(&self, new_rfd_review: NewRfdComment) -> Result; + async fn delete(&self, id: TypedUuid) -> Result, StoreError>; } diff --git a/rfd-model/src/storage/postgres.rs b/rfd-model/src/storage/postgres.rs index 9876d152..d3417eaf 100644 --- a/rfd-model/src/storage/postgres.rs +++ b/rfd-model/src/storage/postgres.rs @@ -22,20 +22,25 @@ use v_model::storage::postgres::PostgresStore; use crate::{ db::{ JobModel, RfdCommentModel, RfdCommentUserModel, RfdModel, RfdPdfModel, - RfdRevisionMetaModel, RfdRevisionModel, + RfdReviewCommentModel, RfdReviewModel, RfdRevisionMetaModel, RfdRevisionModel, + }, + schema::{ + job, rfd, rfd_comment, rfd_comment_user, rfd_pdf, rfd_review, rfd_review_comment, + rfd_revision, }, - schema::{job, rfd, rfd_comment, rfd_comment_user, rfd_pdf, rfd_revision}, schema_ext::Visibility, storage::StoreError, - Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdRevision, Rfd, - RfdComment, RfdCommentId, RfdCommentUser, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdRevision, - RfdRevisionId, RfdRevisionMeta, + Job, NewJob, NewRfd, NewRfdComment, NewRfdCommentUser, NewRfdPdf, NewRfdReview, + NewRfdReviewComment, NewRfdRevision, Rfd, RfdComment, RfdCommentId, RfdCommentUser, + RfdCommentUserId, RfdId, RfdMeta, RfdPdf, RfdPdfId, RfdReview, RfdReviewComment, + RfdReviewCommentId, RfdReviewId, RfdRevision, RfdRevisionId, RfdRevisionMeta, }; use super::{ - JobFilter, JobStore, ListPagination, RfdCommentFilter, RfdCommentStore, RfdCommentUserStore, - RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdRevisionFilter, RfdRevisionMetaStore, - RfdRevisionStore, RfdStore, + JobFilter, JobStore, ListPagination, RfdCommentFilter, RfdCommentStore, RfdCommentUserFilter, + RfdCommentUserStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, + RfdReviewCommentFilter, RfdReviewCommentStore, RfdReviewFilter, RfdReviewStore, + RfdRevisionFilter, RfdRevisionMetaStore, RfdRevisionStore, RfdStore, }; #[async_trait] @@ -756,6 +761,55 @@ impl JobStore for PostgresStore { #[async_trait] impl RfdCommentUserStore for PostgresStore { + async fn get( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + let user = RfdCommentUserStore::list( + self, + vec![RfdCommentUserFilter::default().id(Some(vec![id]))], + &ListPagination::default().limit(1), + ) + .await?; + Ok(user.into_iter().nth(0)) + } + + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + let mut query = rfd_comment_user::table.into_boxed(); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdCommentUserFilter { id } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_comment_user::id + .eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); + } + + let results = query + .offset(pagination.offset) + .limit(pagination.limit) + .get_results_async::(&*self.pool.get().await?) + .await?; + + Ok(results.into_iter().map(|record| record.into()).collect()) + } + async fn upsert( &self, new_rfd_comment_user: NewRfdCommentUser, @@ -763,31 +817,316 @@ impl RfdCommentUserStore for PostgresStore { let user: RfdCommentUserModel = insert_into(rfd_comment_user::table) .values(( rfd_comment_user::id.eq(new_rfd_comment_user.id.into_untyped_uuid()), - rfd_comment_user::github_user_id.eq(new_rfd_comment_user.github_user_id), - rfd_comment_user::github_user_node_id.eq(new_rfd_comment_user.github_user_node_id), - rfd_comment_user::github_user_username - .eq(new_rfd_comment_user.github_user_username), - rfd_comment_user::github_user_avatar_url - .eq(new_rfd_comment_user.github_user_avatar_url), - rfd_comment_user::github_user_type.eq(new_rfd_comment_user.github_user_type), + rfd_comment_user::external_id.eq(new_rfd_comment_user.external_id), + rfd_comment_user::node_id.eq(new_rfd_comment_user.node_id), + rfd_comment_user::user_username.eq(new_rfd_comment_user.user_username), + rfd_comment_user::user_avatar_url.eq(new_rfd_comment_user.user_avatar_url), + rfd_comment_user::user_type.eq(new_rfd_comment_user.user_type), + )) + .on_conflict(rfd_comment_user::external_id) + .do_update() + .set(( + rfd_comment_user::external_id.eq(excluded(rfd_comment_user::external_id)), + rfd_comment_user::node_id.eq(excluded(rfd_comment_user::node_id)), + rfd_comment_user::user_username.eq(excluded(rfd_comment_user::user_username)), + rfd_comment_user::user_avatar_url.eq(excluded(rfd_comment_user::user_avatar_url)), + rfd_comment_user::user_type.eq(excluded(rfd_comment_user::user_type)), + )) + .get_result_async(&*self.pool.get().await?) + .await?; + + Ok(user.into()) + } + + async fn delete( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + let _ = update(rfd_comment_user::table) + .filter(rfd_comment_user::id.eq(id.into_untyped_uuid())) + .set(rfd_comment_user::deleted_at.eq(Utc::now())) + .execute_async(&*self.pool.get().await?) + .await?; + RfdCommentUserStore::get(self, id).await + } +} + +#[async_trait] +impl RfdReviewStore for PostgresStore { + async fn get(&self, id: TypedUuid) -> Result, StoreError> { + let review = RfdReviewStore::list( + self, + vec![RfdReviewFilter::default().id(Some(vec![id]))], + &ListPagination::default().limit(1), + ) + .await?; + Ok(review.into_iter().nth(0)) + } + + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + let mut query = rfd_review::table.into_boxed(); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdReviewFilter { + id, + rfd, + user, + review_created_before, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_review::id.eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(rfd) = rfd { + let rfd_ids = rfd.into_iter().map(GenericUuid::into_untyped_uuid); + predicates.push(Box::new(rfd_review::rfd_id.eq_any(rfd_ids.clone()))); + } + + if let Some(user) = user { + predicates.push(Box::new( + rfd_review::comment_user_id + .eq_any(user.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(review_created_before) = review_created_before { + predicates.push(Box::new( + rfd_review::review_created_at + .assume_not_null() + .le(review_created_before), + )); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); + } + + let results = query + .offset(pagination.offset) + .limit(pagination.limit) + .get_results_async::(&*self.pool.get().await?) + .await?; + + Ok(results.into_iter().map(|record| record.into()).collect()) + } + + async fn upsert(&self, new_rfd_review: NewRfdReview) -> Result { + let user: RfdReviewModel = insert_into(rfd_review::table) + .values(( + rfd_review::id.eq(new_rfd_review.id.into_untyped_uuid()), + rfd_review::rfd_id.eq(new_rfd_review.rfd_id.into_untyped_uuid()), + rfd_review::comment_user_id.eq(new_rfd_review.comment_user_id.into_untyped_uuid()), + rfd_review::external_id.eq(new_rfd_review.external_id), + rfd_review::node_id.eq(new_rfd_review.node_id), + rfd_review::body.eq(new_rfd_review.body), + rfd_review::state.eq(new_rfd_review.state), + rfd_review::commit_id.eq(new_rfd_review.commit_id), + rfd_review::review_created_at.eq(new_rfd_review.review_created_at), )) - .on_conflict(rfd_comment_user::github_user_id) + .on_conflict(rfd_review::external_id) .do_update() .set(( - rfd_comment_user::github_user_id.eq(excluded(rfd_comment_user::github_user_id)), - rfd_comment_user::github_user_node_id - .eq(excluded(rfd_comment_user::github_user_node_id)), - rfd_comment_user::github_user_username - .eq(excluded(rfd_comment_user::github_user_username)), - rfd_comment_user::github_user_avatar_url - .eq(excluded(rfd_comment_user::github_user_avatar_url)), - rfd_comment_user::github_user_type.eq(excluded(rfd_comment_user::github_user_type)), + rfd_review::rfd_id.eq(excluded(rfd_review::rfd_id)), + rfd_review::comment_user_id.eq(excluded(rfd_review::comment_user_id)), + rfd_review::external_id.eq(excluded(rfd_review::external_id)), + rfd_review::node_id.eq(excluded(rfd_review::node_id)), + rfd_review::body.eq(excluded(rfd_review::body)), + rfd_review::state.eq(excluded(rfd_review::state)), + rfd_review::commit_id.eq(excluded(rfd_review::commit_id)), + rfd_review::review_created_at.eq(excluded(rfd_review::review_created_at)), )) .get_result_async(&*self.pool.get().await?) .await?; Ok(user.into()) } + + async fn delete(&self, id: TypedUuid) -> Result, StoreError> { + let _ = update(rfd_review::table) + .filter(rfd_review::id.eq(id.into_untyped_uuid())) + .set(rfd_review::deleted_at.eq(Utc::now())) + .execute_async(&*self.pool.get().await?) + .await?; + RfdReviewStore::get(self, id).await + } +} + +#[async_trait] +impl RfdReviewCommentStore for PostgresStore { + async fn get( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + let comment = RfdReviewCommentStore::list( + self, + vec![RfdReviewCommentFilter::default().id(Some(vec![id]))], + &ListPagination::default().limit(1), + ) + .await?; + Ok(comment.into_iter().nth(0)) + } + + async fn list( + &self, + filters: Vec, + pagination: &ListPagination, + ) -> Result, StoreError> { + let mut query = rfd_review_comment::table.into_boxed(); + let filter_predicates = filters + .into_iter() + .map(|filter| { + let mut predicates: Vec>> = vec![]; + let RfdReviewCommentFilter { + id, + rfd, + user, + review, + comment_created_before, + } = filter; + + if let Some(id) = id { + predicates.push(Box::new( + rfd_review_comment::id + .eq_any(id.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(rfd) = rfd { + let rfd_ids = rfd.into_iter().map(GenericUuid::into_untyped_uuid); + predicates.push(Box::new(rfd_review_comment::rfd_id.eq_any(rfd_ids.clone()))); + } + + if let Some(user) = user { + predicates.push(Box::new( + rfd_review_comment::comment_user_id + .eq_any(user.into_iter().map(GenericUuid::into_untyped_uuid)), + )); + } + + if let Some(review) = review { + let review_ids = review.into_iter().map(GenericUuid::into_untyped_uuid); + predicates.push(Box::new( + rfd_review_comment::review_id.eq_any(review_ids.clone()), + )); + } + + if let Some(comment_created_before) = comment_created_before { + predicates.push(Box::new( + rfd_review_comment::comment_created_at + .assume_not_null() + .le(comment_created_before), + )); + } + + predicates + }) + .collect::>(); + + if let Some(predicate) = flatten_predicates(filter_predicates) { + query = query.filter(predicate); + } + + let results = query + .offset(pagination.offset) + .limit(pagination.limit) + .get_results_async::(&*self.pool.get().await?) + .await?; + + Ok(results.into_iter().map(|record| record.into()).collect()) + } + + async fn upsert( + &self, + new_comment: NewRfdReviewComment, + ) -> Result { + let comment: RfdReviewCommentModel = insert_into(rfd_review_comment::table) + .values(( + rfd_review_comment::id.eq(new_comment.id.into_untyped_uuid()), + rfd_review_comment::rfd_id.eq(new_comment.rfd_id.into_untyped_uuid()), + rfd_review_comment::comment_user_id + .eq(new_comment.comment_user_id.into_untyped_uuid()), + rfd_review_comment::external_id.eq(new_comment.external_id), + rfd_review_comment::node_id.eq(new_comment.node_id), + rfd_review_comment::review_id + .eq(new_comment.review_id.map(GenericUuid::into_untyped_uuid)), + rfd_review_comment::diff_hunk.eq(new_comment.diff_hunk), + rfd_review_comment::path.eq(new_comment.path), + rfd_review_comment::body.eq(new_comment.body), + rfd_review_comment::commit_id.eq(new_comment.commit_id), + rfd_review_comment::original_commit_id.eq(new_comment.original_commit_id), + rfd_review_comment::line.eq(new_comment.line), + rfd_review_comment::original_line.eq(new_comment.original_line), + rfd_review_comment::start_line.eq(new_comment.start_line), + rfd_review_comment::original_start_line.eq(new_comment.original_start_line), + rfd_review_comment::side.eq(new_comment.side), + rfd_review_comment::start_side.eq(new_comment.start_side), + rfd_review_comment::subject.eq(new_comment.subject), + rfd_review_comment::in_reply_to.eq(new_comment.in_reply_to), + rfd_review_comment::comment_created_at.eq(new_comment.comment_created_at), + rfd_review_comment::comment_updated_at.eq(new_comment.comment_updated_at), + )) + .on_conflict(rfd_review_comment::external_id) + .do_update() + .set(( + rfd_review_comment::comment_user_id + .eq(excluded(rfd_review_comment::comment_user_id)), + rfd_review_comment::external_id.eq(excluded(rfd_review_comment::external_id)), + rfd_review_comment::node_id.eq(excluded(rfd_review_comment::node_id)), + rfd_review_comment::review_id.eq(excluded(rfd_review_comment::review_id)), + rfd_review_comment::diff_hunk.eq(excluded(rfd_review_comment::diff_hunk)), + rfd_review_comment::path.eq(excluded(rfd_review_comment::path)), + rfd_review_comment::body.eq(excluded(rfd_review_comment::body)), + rfd_review_comment::commit_id.eq(excluded(rfd_review_comment::commit_id)), + rfd_review_comment::original_commit_id + .eq(excluded(rfd_review_comment::original_commit_id)), + rfd_review_comment::line.eq(excluded(rfd_review_comment::line)), + rfd_review_comment::original_line.eq(excluded(rfd_review_comment::original_line)), + rfd_review_comment::start_line.eq(excluded(rfd_review_comment::start_line)), + rfd_review_comment::original_start_line + .eq(excluded(rfd_review_comment::original_start_line)), + rfd_review_comment::side.eq(excluded(rfd_review_comment::side)), + rfd_review_comment::start_side.eq(excluded(rfd_review_comment::start_side)), + rfd_review_comment::subject.eq(excluded(rfd_review_comment::subject)), + rfd_review_comment::in_reply_to.eq(excluded(rfd_review_comment::in_reply_to)), + rfd_review_comment::comment_created_at + .eq(excluded(rfd_review_comment::comment_created_at)), + rfd_review_comment::comment_updated_at + .eq(excluded(rfd_review_comment::comment_updated_at)), + )) + .get_result_async(&*self.pool.get().await?) + .await?; + + Ok( + RfdReviewCommentStore::get(self, TypedUuid::from_untyped_uuid(comment.id)) + .await? + .expect("Upserted comment must exist"), + ) + } + + async fn delete( + &self, + id: TypedUuid, + ) -> Result, StoreError> { + let _ = update(rfd_review_comment::table) + .filter(rfd_review_comment::id.eq(id.into_untyped_uuid())) + .set(rfd_review_comment::deleted_at.eq(Utc::now())) + .execute_async(&*self.pool.get().await?) + .await?; + RfdReviewCommentStore::get(self, id).await + } } #[async_trait] @@ -807,9 +1146,7 @@ impl RfdCommentStore for PostgresStore { filters: Vec, pagination: &ListPagination, ) -> Result, StoreError> { - let mut query = rfd_comment::table - .inner_join(rfd_comment_user::table) - .into_boxed(); + let mut query = rfd_comment::table.into_boxed(); let filter_predicates = filters .into_iter() .map(|filter| { @@ -828,10 +1165,8 @@ impl RfdCommentStore for PostgresStore { } if let Some(rfd) = rfd { - predicates.push(Box::new( - rfd_comment::rfd_id - .eq_any(rfd.into_iter().map(GenericUuid::into_untyped_uuid)), - )); + let rfd_ids = rfd.into_iter().map(GenericUuid::into_untyped_uuid); + predicates.push(Box::new(rfd_comment::rfd_id.eq_any(rfd_ids.clone()))); } if let Some(user) = user { @@ -860,57 +1195,34 @@ impl RfdCommentStore for PostgresStore { let results = query .offset(pagination.offset) .limit(pagination.limit) - .get_results_async::<(RfdCommentModel, RfdCommentUserModel)>(&*self.pool.get().await?) + .get_results_async::(&*self.pool.get().await?) .await?; Ok(results.into_iter().map(|record| record.into()).collect()) } - async fn upsert(&self, new_rfd_comment: NewRfdComment) -> Result { + async fn upsert(&self, new_comment: NewRfdComment) -> Result { let comment: RfdCommentModel = insert_into(rfd_comment::table) .values(( - rfd_comment::id.eq(new_rfd_comment.id.into_untyped_uuid()), - rfd_comment::rfd_id.eq(new_rfd_comment.rfd_id.into_untyped_uuid()), - rfd_comment::comment_user_id.eq(new_rfd_comment.comment_user.into_untyped_uuid()), - rfd_comment::external_id.eq(new_rfd_comment.external_id), - rfd_comment::node_id.eq(new_rfd_comment.node_id), - rfd_comment::discussion_number.eq(new_rfd_comment.discussion_number), - rfd_comment::diff_hunk.eq(new_rfd_comment.diff_hunk), - rfd_comment::path.eq(new_rfd_comment.path), - rfd_comment::body.eq(new_rfd_comment.body), - rfd_comment::commit_id.eq(new_rfd_comment.commit_id), - rfd_comment::original_commit_id.eq(new_rfd_comment.original_commit_id), - rfd_comment::line.eq(new_rfd_comment.line), - rfd_comment::original_line.eq(new_rfd_comment.original_line), - rfd_comment::start_line.eq(new_rfd_comment.start_line), - rfd_comment::original_start_line.eq(new_rfd_comment.original_start_line), - rfd_comment::side.eq(new_rfd_comment.side), - rfd_comment::start_side.eq(new_rfd_comment.start_side), - rfd_comment::subject.eq(new_rfd_comment.subject), - rfd_comment::in_reply_to.eq(new_rfd_comment.in_reply_to), - rfd_comment::comment_created_at.eq(new_rfd_comment.comment_created_at), - rfd_comment::comment_updated_at.eq(new_rfd_comment.comment_updated_at), + rfd_comment::id.eq(new_comment.id.into_untyped_uuid()), + rfd_comment::rfd_id.eq(new_comment.rfd_id.into_untyped_uuid()), + rfd_comment::comment_user_id.eq(new_comment.comment_user_id.into_untyped_uuid()), + rfd_comment::external_id.eq(new_comment.external_id), + rfd_comment::node_id.eq(new_comment.node_id), + rfd_comment::body.eq(new_comment.body), + rfd_comment::comment_created_at.eq(new_comment.comment_created_at), + rfd_comment::comment_updated_at.eq(new_comment.comment_updated_at), )) .on_conflict(rfd_comment::external_id) .do_update() .set(( + rfd_comment::rfd_id.eq(new_comment.rfd_id.into_untyped_uuid()), rfd_comment::comment_user_id.eq(excluded(rfd_comment::comment_user_id)), rfd_comment::external_id.eq(excluded(rfd_comment::external_id)), rfd_comment::node_id.eq(excluded(rfd_comment::node_id)), - rfd_comment::discussion_number.eq(excluded(rfd_comment::discussion_number)), - rfd_comment::diff_hunk.eq(excluded(rfd_comment::diff_hunk)), - rfd_comment::path.eq(excluded(rfd_comment::path)), rfd_comment::body.eq(excluded(rfd_comment::body)), - rfd_comment::commit_id.eq(excluded(rfd_comment::commit_id)), - rfd_comment::original_commit_id.eq(excluded(rfd_comment::original_commit_id)), - rfd_comment::line.eq(excluded(rfd_comment::line)), - rfd_comment::original_line.eq(excluded(rfd_comment::original_line)), - rfd_comment::start_line.eq(excluded(rfd_comment::start_line)), - rfd_comment::original_start_line.eq(excluded(rfd_comment::original_start_line)), - rfd_comment::side.eq(excluded(rfd_comment::side)), - rfd_comment::start_side.eq(excluded(rfd_comment::start_side)), - rfd_comment::subject.eq(excluded(rfd_comment::subject)), - rfd_comment::in_reply_to.eq(excluded(rfd_comment::in_reply_to)), + rfd_comment::comment_created_at.eq(excluded(rfd_comment::comment_created_at)), + rfd_comment::comment_updated_at.eq(excluded(rfd_comment::comment_updated_at)), )) .get_result_async(&*self.pool.get().await?) .await?; @@ -922,13 +1234,13 @@ impl RfdCommentStore for PostgresStore { ) } - async fn delete(&self, id: &TypedUuid) -> Result<(), StoreError> { + async fn delete(&self, id: TypedUuid) -> Result, StoreError> { let _ = update(rfd_comment::table) .filter(rfd_comment::id.eq(id.into_untyped_uuid())) .set(rfd_comment::deleted_at.eq(Utc::now())) .execute_async(&*self.pool.get().await?) .await?; - Ok(()) + RfdCommentStore::get(self, id).await } } From d340273bf699890ad3ecd1576bb58f874ced8cb1 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Mon, 23 Dec 2024 09:24:34 -0600 Subject: [PATCH 15/17] Update comments endpoint --- rfd-api/src/endpoints/rfd.rs | 10 +++++----- rfd-api/src/server.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 4b766530..5c276b92 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -169,10 +169,10 @@ pub async fn view_rfd_attr( #[trace_request] #[endpoint { method = GET, - path = "/rfd/{number}/comments", + path = "/rfd/{number}/discussion", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn view_rfd_comments( +pub async fn view_rfd_discussion( rqctx: RequestContext, path: Path, ) -> Result, HttpError> { @@ -260,10 +260,10 @@ pub async fn view_rfd_revision_attr( #[trace_request] #[endpoint { method = GET, - path = "/rfd/{number}/revision/{revision}/comments", + path = "/rfd/{number}/revision/{revision}/discussion", }] #[instrument(skip(rqctx), fields(request_id = rqctx.request_id), err(Debug))] -pub async fn view_rfd_revision_comments( +pub async fn view_rfd_revision_discussion( rqctx: RequestContext, path: Path, ) -> Result, HttpError> { @@ -422,7 +422,7 @@ async fn view_rfd_attr_op( } #[instrument(skip(ctx, caller), fields(caller = ?caller.id), err(Debug))] -async fn view_rfd_comments_op( +async fn view_rfd_discussion_op( ctx: &RfdContext, caller: &Caller, number: String, diff --git a/rfd-api/src/server.rs b/rfd-api/src/server.rs index 18080922..7d45e25f 100644 --- a/rfd-api/src/server.rs +++ b/rfd-api/src/server.rs @@ -18,8 +18,8 @@ use crate::{ rfd::{ discuss_rfd, list_rfds, publish_rfd, reserve_rfd, search_rfds, set_rfd_attr, set_rfd_content, set_rfd_document, update_rfd_visibility, view_rfd, view_rfd_attr, - view_rfd_comments, view_rfd_meta, view_rfd_pdf, view_rfd_revision, - view_rfd_revision_attr, view_rfd_revision_comments, view_rfd_revision_meta, + view_rfd_discussion, view_rfd_meta, view_rfd_pdf, view_rfd_revision, + view_rfd_revision_attr, view_rfd_revision_discussion, view_rfd_revision_meta, view_rfd_revision_pdf, }, webhook::github_webhook, @@ -86,7 +86,7 @@ pub fn server( .expect("Failed to register endpoint"); api.register(view_rfd_attr) .expect("Failed to register endpoint"); - api.register(view_rfd_comments) + api.register(view_rfd_discussion) .expect("Failed to register endpoint"); api.register(view_rfd_revision_meta) @@ -97,7 +97,7 @@ pub fn server( .expect("Failed to register endpoint"); api.register(view_rfd_revision_attr) .expect("Failed to register endpoint"); - api.register(view_rfd_revision_comments) + api.register(view_rfd_revision_discussion) .expect("Failed to register endpoint"); api.register(search_rfds) From 142658eba76d6b8bcb512e2c0a891a3303c9aa63 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Mon, 23 Dec 2024 09:51:58 -0600 Subject: [PATCH 16/17] Fix discussion op call --- rfd-api/src/endpoints/rfd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index 2c9485fc..f2efbf59 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -179,7 +179,7 @@ pub async fn view_rfd_discussion( let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); - view_rfd_comments_op(ctx, &caller, path.number, None).await + view_rfd_discussion_op(ctx, &caller, path.number, None).await } // Specific RFD revision endpoints @@ -273,7 +273,7 @@ pub async fn view_rfd_revision_discussion( let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); - view_rfd_comments_op(ctx, &caller, path.number, Some(path.revision.into())).await + view_rfd_discussion_op(ctx, &caller, path.number, Some(path.revision.into())).await } #[derive(Debug, Deserialize, JsonSchema)] From 4e32336100c2cdc6c86f1dd4c3c723de9d583f70 Mon Sep 17 00:00:00 2001 From: Augustus Mayo Date: Mon, 23 Dec 2024 10:02:06 -0600 Subject: [PATCH 17/17] Return discussion struct --- rfd-api/src/context.rs | 61 ++++++++++++++++++++++++++++++------ rfd-api/src/endpoints/rfd.rs | 16 ++++++---- rfd-model/src/storage/mod.rs | 4 +++ 3 files changed, 66 insertions(+), 15 deletions(-) diff --git a/rfd-api/src/context.rs b/rfd-api/src/context.rs index 7214c8e8..cefe74d0 100644 --- a/rfd-api/src/context.rs +++ b/rfd-api/src/context.rs @@ -18,11 +18,12 @@ use rfd_github::{GitHubError, GitHubNewRfdNumber, GitHubRfdRepo}; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, storage::{ - JobStore, RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdReviewCommentFilter, - RfdReviewCommentStore, RfdStorage, RfdStore, + JobStore, RfdCommentFilter, RfdCommentStore, RfdCommentUserFilter, RfdCommentUserStore, + RfdFilter, RfdMetaStore, RfdPdfFilter, RfdPdfStore, RfdReviewCommentFilter, + RfdReviewCommentStore, RfdReviewFilter, RfdReviewStore, RfdStorage, RfdStore, }, - CommitSha, FileSha, Job, NewJob, Rfd, RfdId, RfdMeta, RfdPdf, RfdReviewComment, RfdRevision, - RfdRevisionId, + CommitSha, FileSha, Job, NewJob, Rfd, RfdComment, RfdCommentUser, RfdId, RfdMeta, RfdPdf, + RfdReview, RfdReviewComment, RfdRevision, RfdRevisionId, }; use rsa::{ pkcs1::{DecodeRsaPrivateKey, EncodeRsaPrivateKey}, @@ -177,6 +178,14 @@ impl From> for RfdRevisionIdentifier { } } +#[derive(Debug, Clone, Deserialize, Serialize, JsonSchema)] +pub struct RfdDiscussion { + pub users: Vec, + pub reviews: Vec, + pub review_comments: Vec, + pub comments: Vec, +} + impl RfdContext { pub async fn new( public_url: String, @@ -516,23 +525,57 @@ impl RfdContext { Ok(pdfs) } - pub async fn view_rfd_comments( + pub async fn view_rfd_discussion( &self, caller: &Caller, rfd_number: i32, revision: Option, - ) -> ResourceResult, StoreError> { + ) -> ResourceResult { let rfd = self.get_rfd_meta(caller, rfd_number, revision).await?; - let comments = RfdReviewCommentStore::list( + let reviews = RfdReviewStore::list( + &*self.storage, + vec![RfdReviewFilter::default().rfd(Some(vec![rfd.id]))], + &ListPagination::unlimited(), + ) + .await + .to_resource_result()?; + let review_ids = reviews.iter().map(|review| review.id).collect::>(); + let user_ids = reviews + .iter() + .map(|review| review.comment_user_id) + .collect::>(); + let users = RfdCommentUserStore::list( + &*self.storage, + vec![RfdCommentUserFilter::default().id(Some(user_ids.clone()))], + &ListPagination::unlimited(), + ) + .await + .to_resource_result()?; + let review_comments = RfdReviewCommentStore::list( &*self.storage, vec![RfdReviewCommentFilter::default() .rfd(Some(vec![rfd.id])) - .comment_created_before(Some(rfd.content.committed_at))], + .user(Some(user_ids.clone())) + .review(Some(review_ids))], + &ListPagination::unlimited(), + ) + .await + .to_resource_result()?; + let comments = RfdCommentStore::list( + &*self.storage, + vec![RfdCommentFilter::default() + .rfd(Some(vec![rfd.id])) + .user(Some(user_ids))], &ListPagination::unlimited(), ) .await .to_resource_result()?; - Ok(comments) + Ok(RfdDiscussion { + users, + reviews, + review_comments, + comments, + }) } #[instrument(skip(self, caller, content))] diff --git a/rfd-api/src/endpoints/rfd.rs b/rfd-api/src/endpoints/rfd.rs index f2efbf59..26420541 100644 --- a/rfd-api/src/endpoints/rfd.rs +++ b/rfd-api/src/endpoints/rfd.rs @@ -13,7 +13,7 @@ use rfd_data::{ }; use rfd_model::{ schema_ext::{ContentFormat, Visibility}, - Rfd, RfdPdf, RfdReviewComment, RfdRevisionId, + Rfd, RfdPdf, RfdRevisionId, }; use schemars::JsonSchema; use serde::{Deserialize, Serialize}; @@ -24,7 +24,9 @@ use v_model::permissions::Caller; use crate::{ caller::CallerExt, - context::{RfdContext, RfdRevisionIdentifier, RfdWithContent, RfdWithoutContent}, + context::{ + RfdContext, RfdDiscussion, RfdRevisionIdentifier, RfdWithContent, RfdWithoutContent, + }, permissions::RfdPermission, search::{MeiliSearchResult, SearchRequest}, util::response::{client_error, internal_error, unauthorized}, @@ -175,7 +177,7 @@ pub async fn view_rfd_attr( pub async fn view_rfd_discussion( rqctx: RequestContext, path: Path, -) -> Result>, HttpError> { +) -> Result, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); @@ -269,7 +271,7 @@ pub async fn view_rfd_revision_attr( pub async fn view_rfd_revision_discussion( rqctx: RequestContext, path: Path, -) -> Result>, HttpError> { +) -> Result, HttpError> { let ctx = rqctx.context(); let caller = ctx.v_ctx().get_caller(&rqctx).await?; let path = path.into_inner(); @@ -433,9 +435,11 @@ async fn view_rfd_discussion_op( caller: &Caller, number: String, revision: Option, -) -> Result>, HttpError> { +) -> Result, HttpError> { if let Ok(rfd_number) = number.parse::() { - let comments = ctx.view_rfd_comments(caller, rfd_number, revision).await?; + let comments = ctx + .view_rfd_discussion(caller, rfd_number, revision) + .await?; Ok(HttpResponseOk(comments)) } else { Err(client_error( diff --git a/rfd-model/src/storage/mod.rs b/rfd-model/src/storage/mod.rs index 5a6cc68a..a7547862 100644 --- a/rfd-model/src/storage/mod.rs +++ b/rfd-model/src/storage/mod.rs @@ -31,7 +31,9 @@ pub trait RfdStorage: + RfdPdfStore + JobStore + RfdCommentUserStore + + RfdReviewStore + RfdReviewCommentStore + + RfdCommentStore + Send + Sync + 'static @@ -45,7 +47,9 @@ impl RfdStorage for T where + RfdPdfStore + JobStore + RfdCommentUserStore + + RfdReviewStore + RfdReviewCommentStore + + RfdCommentStore + Send + Sync + 'static