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.

Sign up for GitHub

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

enhancement(config api): Add graphql field to toggle graphql endpoint #19645

Merged
merged 10 commits into from
Jan 23, 2024
Merged

enhancement(config api): Add graphql field to toggle graphql endpoint #19645

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
7f2e52f
OPW-195
Jan 17, 2024
33e02f5
knit: param change
Jan 18, 2024
d1c5c5c
docs
Jan 18, 2024
e7678ff
trailing whitespaces
Jan 18, 2024
cc933b2
rustfmt
Jan 18, 2024
43e56df
docs and test fix
Jan 18, 2024
993d0ea
serialization skip for new field & typing
Jan 19, 2024
976206a
passby clippy
Jan 19, 2024
5a9e157
spacing
Jan 19, 2024
690bb2b
nit
Jan 22, 2024
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
23 changes: 14 additions & 9 deletions src/api/server.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl Server {
running: Arc<AtomicBool>,
handle: &Handle,
) -> crate::Result<Self> {
let routes = make_routes(config.api.playground, watch_rx, running);
let routes = make_routes(config.api.graphql, config.api.playground, watch_rx, running);
sebastiantia marked this conversation as resolved.
Show resolved Hide resolved

let (_shutdown, rx) = oneshot::channel();
// warp uses `tokio::spawn` and so needs us to enter the runtime context.
Expand Down Expand Up @@ -96,6 +96,7 @@ impl Server {
}

fn make_routes(
graphql: bool,
playground: bool,
watch_tx: topology::WatchRx,
running: Arc<AtomicBool>,
Expand Down Expand Up @@ -140,16 +141,20 @@ fn make_routes(
// Handle GraphQL queries. Headers will first be parsed to determine whether the query is
// a subscription and if so, an attempt will be made to upgrade the connection to WebSockets.
// All other queries will fall back to the default HTTP handler.
let graphql_handler = warp::path("graphql").and(graphql_subscription_handler.or(
async_graphql_warp::graphql(schema::build_schema().finish()).and_then(
|(schema, request): (Schema<_, _, _>, Request)| async move {
Ok::<_, Infallible>(GraphQLResponse::from(schema.execute(request).await))
},
),
));
let graphql_handler = if graphql {
warp::path("graphql").and(graphql_subscription_handler.or(
async_graphql_warp::graphql(schema::build_schema().finish()).and_then(
|(schema, request): (Schema<_, _, _>, Request)| async move {
Ok::<_, Infallible>(GraphQLResponse::from(schema.execute(request).await))
},
),
)).boxed()
} else {
warp::any().and_then(|| async { Err(warp::reject::not_found()) }).boxed()
};

// Provide a playground for executing GraphQL queries/mutations/subscriptions.
let graphql_playground = if playground {
let graphql_playground = if playground & graphql {
warp::path("playground")
.map(move || {
Response::builder()
Expand Down
14 changes: 14 additions & 0 deletions src/config/api.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ pub struct Options {
/// Whether or not to expose the GraphQL playground on the API endpoint.
#[serde(default = "default_playground")]
pub playground: bool,

/// Whether or not the GraphQL endpoint is enabled
#[serde(default = "default_graphql")]
Copy link
Member

Choose a reason for hiding this comment

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

Since this default is not the type-level default, and we depend on that not being changing the serialized form, this needs an additional attribute:

Suggested change
#[serde(default = "default_graphql")]
#[serde(default = "default_graphql", skip_serializing_if = "is_true")]

along with an additional function of:

fn is_true(field: &bool) -> bool {
    *field
}

(Normally, you could use just #[serde(skip_serializing_if = "vector_lib::serde::is_default")], but that only works if the default value is the type default, which here it is not. See also https://serde.rs/field-attrs.html#skip_serializing_if

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Awesome, thanks for the detail! Not super familiar with the serde framework so I appreciate the help!

pub graphql: bool,
}

impl Default for Options {
Expand All @@ -27,6 +31,7 @@ impl Default for Options {
enabled: default_enabled(),
playground: default_playground(),
address: default_address(),
graphql: default_graphql(),
}
}
}
Expand All @@ -53,6 +58,10 @@ const fn default_playground() -> bool {
true
}

const fn default_graphql() -> bool {
true
}

impl Options {
pub fn merge(&mut self, other: Self) -> Result<(), String> {
// Merge options
Expand All @@ -78,6 +87,7 @@ impl Options {
address,
enabled: self.enabled | other.enabled,
playground: self.playground & other.playground,
graphql: self.graphql & other.graphql,
};

*self = options;
Expand All @@ -91,6 +101,7 @@ fn bool_merge() {
enabled: true,
address: None,
playground: false,
graphql: false,
};

a.merge(Options::default()).unwrap();
Expand All @@ -101,6 +112,7 @@ fn bool_merge() {
enabled: true,
address: default_address(),
playground: false,
graphql: false
}
);
}
Expand All @@ -112,6 +124,7 @@ fn bind_merge() {
enabled: true,
address: Some(address),
playground: true,
graphql: true,
};

a.merge(Options::default()).unwrap();
Expand All @@ -122,6 +135,7 @@ fn bind_merge() {
enabled: true,
address: Some(address),
playground: true,
graphql: true,
}
);
}
Expand Down
Loading