Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add updateTrustedDomain GraphQL API #276

Merged
merged 1 commit into from
Dec 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,11 @@ Versioning](https://semver.org/spec/v2.0.0.html).

## [Unreleased]

### Added

- Added the `updateTrustedDomain` GraphQL API, allowing users to modify a
trusted domain.

### Changed

- Updated the encoding and decoding method for GraphQL cursors by removing
Expand Down
1 change: 1 addition & 0 deletions src/graphql.rs
Original file line number Diff line number Diff line change
Expand Up @@ -716,6 +716,7 @@ mod tests {
#[tokio::test]
async fn unimplemented_agent_manager() {
let agent_manager = super::MockAgentManager {};
assert!(agent_manager.broadcast_trusted_domains().await.is_ok());
assert!(agent_manager
.broadcast_trusted_user_agent_list(&[])
.await
Expand Down
88 changes: 86 additions & 2 deletions src/graphql/trusted_domain.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use async_graphql::connection::OpaqueCursor;
use async_graphql::{
connection::{Connection, EmptyFields},
Context, Object, Result, SimpleObject,
Context, InputObject, Object, Result, SimpleObject,
};

use super::{AgentManager, BoxedAgentManager, Role, RoleGuard};
Expand Down Expand Up @@ -63,6 +63,29 @@ impl TrustedDomainMutation {
Ok(name)
}

/// Updates a trusted domain, returning the new value.
#[graphql(guard = "RoleGuard::new(Role::SystemAdministrator)
.or(RoleGuard::new(Role::SecurityAdministrator))")]
async fn update_trusted_domain(
&self,
ctx: &Context<'_>,
old: TrustedDomainInput,
new: TrustedDomainInput,
) -> Result<String> {
let name = {
let store = crate::graphql::get_store(ctx).await?;
let map = store.trusted_domain_map();
let old = review_database::TrustedDomain::from(old);
let new = review_database::TrustedDomain::from(new);
map.update(&old, &new)?;
new.name
};
Comment on lines +75 to +82
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that your style, where a variable is declared early and assigned a specific value later, is still being used. I believe this approach is not ideal. Moreover, in this context, the name variable doesn't appear to be essential.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have any specific expectations for the return type of the GraphQL API?
It seems that Result<()> cannot be used.

42 | #[Object]
   | ^^^^^^^^^ the trait `OutputType` is not implemented for `()`

Are you considering declaring and using a custom scalar type like Void?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I apologize for my confusion. I thought this was similar to what you had done before, but it seems inevitable for the later asynchronous code. Once again, I’m sorry for my mistake in not catching the later part.


let agent_manager = ctx.data::<BoxedAgentManager>()?;
sophie-cluml marked this conversation as resolved.
Show resolved Hide resolved
agent_manager.broadcast_trusted_domains().await?;
Ok(name)
}

/// Removes a trusted domain, returning the old value if it existed.
#[graphql(
guard = "RoleGuard::new(Role::SystemAdministrator).or(RoleGuard::new(Role::SecurityAdministrator))"
Expand Down Expand Up @@ -95,6 +118,21 @@ impl From<review_database::TrustedDomain> for TrustedDomain {
}
}

#[derive(InputObject)]
pub(super) struct TrustedDomainInput {
name: String,
remarks: String,
}

impl From<TrustedDomainInput> for review_database::TrustedDomain {
fn from(input: TrustedDomainInput) -> Self {
Self {
name: input.name,
remarks: input.remarks,
}
}
}

async fn load(
ctx: &Context<'_>,
after: Option<OpaqueCursor<Vec<u8>>>,
Expand All @@ -109,7 +147,9 @@ async fn load(

#[cfg(test)]
mod tests {
use crate::graphql::TestSchema;
use std::net::SocketAddr;

use crate::graphql::{BoxedAgentManager, MockAgentManager, TestSchema};

#[tokio::test]
async fn trusted_domain_list() {
Expand Down Expand Up @@ -158,4 +198,48 @@ mod tests {
r#"{trustedDomainList: {edges: [{node: {name: "example2.org"}}]}}"#
);
}

#[tokio::test]
async fn update_trusted_domain() {
let agent_manager: BoxedAgentManager = Box::new(MockAgentManager {});
let test_addr: SocketAddr = "127.0.0.1:8080".parse().unwrap();
let schema = TestSchema::new_with(agent_manager, Some(test_addr)).await;
let insert_query = r#"
mutation {
insertTrustedDomain(
name: "test.com"
remarks: "origin_remarks"
)
}
"#;
let update_query = r#"
mutation {
updateTrustedDomain(
old: {
name: "test.com"
remarks: "origin_remarks"
}
new: {
name: "test2.com"
remarks: "updated_remarks"
}
)
}
"#;

let res = schema.execute(update_query).await;
assert_eq!(
res.errors.first().unwrap().message,
"no such entry".to_string()
);

let res = schema.execute(insert_query).await;
assert_eq!(res.data.to_string(), r#"{insertTrustedDomain: "test.com"}"#);

let res = schema.execute(update_query).await;
assert_eq!(
res.data.to_string(),
r#"{updateTrustedDomain: "test2.com"}"#
);
}
}
Loading