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

fix: indent and commas #101

Merged
merged 8 commits into from
Nov 18, 2021
Merged

fix: indent and commas #101

Changes from 6 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ac91bf8
fix: indent and commas
o0Ignition0o Nov 8, 2021
bb592e3
ok clippy
o0Ignition0o Nov 8, 2021
b51df81
add failing tests so i can understand what to do exactly with the error
o0Ignition0o Nov 9, 2021
033a241
ok this should do
o0Ignition0o Nov 9, 2021
9d1b032
thank you so much Geoffroy for adding tests that prevented us from ha…
o0Ignition0o Nov 9, 2021
e8284d2
yay workaround \o/
o0Ignition0o Nov 9, 2021
630ef5e
bump apollo-rs and use the new convenience method
o0Ignition0o Nov 17, 2021
c4979d1
Merge branch 'main' into igni/fix_indent_and_commas
o0Ignition0o Nov 17, 2021
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
50 changes: 40 additions & 10 deletions crates/apollo-router-core/src/schema.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::*;
use apollo_parser::ast;
use apollo_parser::ast::{self, AstNode};
use std::collections::{HashMap, HashSet};

#[derive(Debug)]
Expand Down Expand Up @@ -117,11 +117,21 @@ impl std::str::FromStr for Schema {

let arg_value =
argument.value().and_then(|s| {
s.to_string()
.trim_end()
.strip_prefix('"')
.and_then(|s| s.strip_suffix('"'))
.map(|s| s.to_owned())
// This is a temporary workaround until we have nice semantic analysis
s.syntax()
.green()
.children()
.next()
.and_then(|it| it.into_token())
.map(|token| {
token
.text()
.trim()
.trim_start_matches('"')
.trim_end_matches('"')
.trim()
.to_string()
})
});
Copy link
Member

Choose a reason for hiding this comment

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

if you update to the latest apollo-rs commit you can get the STRING_VALUE as rust's String type:

if let ast::Value::StringValue(val) =
    argument.value().expect("Cannot get argument value.")
{
    let s: String = val.into();
}

It should help you eliminate this green node manipulation happening here.

Copy link
Member

Choose a reason for hiding this comment

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

^ It is still preferable that you match on the variants of argument.value(), as not all variants implement casting to native rust types (they are not likely to in the future either).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it works! 🙌 thank you! 🚀


match arg_name.as_deref() {
Expand All @@ -131,7 +141,6 @@ impl std::str::FromStr for Schema {
};
}
}

if let (Some(name), Some(url)) = (name, url) {
// FIXME: return an error on name collisions
subgraphs.insert(name, url);
Expand Down Expand Up @@ -228,8 +237,10 @@ mod tests {

enum join__Graph {
ACCOUNTS @join__graph(name:"accounts" url: "http://localhost:4001/graphql")
INVENTORY @join__graph(name: "inventory" url: "http://localhost:4004/graphql")
PRODUCTS @join__graph(name: "products" url: "http://localhost:4003/graphql")
INVENTORY
@join__graph(name: "inventory", url: "http://localhost:4004/graphql")
PRODUCTS
@join__graph(name: "products" url: "http://localhost:4003/graphql")
REVIEWS @join__graph(name: "reviews" url: "http://localhost:4002/graphql")
}"#
.parse()
Expand All @@ -239,7 +250,26 @@ mod tests {
assert_eq!(schema.subgraphs.len(), 4);
assert_eq!(
schema.subgraphs.get("accounts").map(|s| s.as_str()),
Some("http://localhost:4001/graphql")
Some("http://localhost:4001/graphql"),
"Incorrect url for accounts"
);

assert_eq!(
schema.subgraphs.get("inventory").map(|s| s.as_str()),
Some("http://localhost:4004/graphql"),
"Incorrect url for inventory"
);

assert_eq!(
schema.subgraphs.get("products").map(|s| s.as_str()),
Some("http://localhost:4003/graphql"),
"Incorrect url for products"
);

assert_eq!(
schema.subgraphs.get("reviews").map(|s| s.as_str()),
Some("http://localhost:4002/graphql"),
"Incorrect url for reviews"
);

assert_eq!(schema.subgraphs.get("test"), None);
Expand Down