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

Rust implementation of the core spec #326

Closed
wants to merge 38 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
3b396bd
Using crate
Jan 13, 2021
044a6bf
Support crate for using directive, move parser and query-planner back…
Jan 13, 2021
611457d
Rename the various ParseErrors for clarity.
Jan 13, 2021
eadb045
More test cases and overlapping prefix validation.
Jan 13, 2021
2c491d8
Add tests for no schemas and too many schemas.
Jan 13, 2021
bb8a351
Formatting for version, better docs and error handling in Schema
Jan 14, 2021
7a9fa19
Version specifier tests.
Jan 14, 2021
2add884
Docs and tests for version module
Jan 14, 2021
b5cc83a
Documentation for request module.
Jan 14, 2021
f7c9d82
docs for constants and specs
Jan 14, 2021
6595613
More doc comments for spec.
Jan 14, 2021
f410fe3
Implementations module.
Jan 14, 2021
b0940d1
Activating implementations.
Jan 14, 2021
674dd0a
Example of using Fns as implementations.
Jan 14, 2021
9103cff
Remove unused import.
Jan 14, 2021
bfc8d9e
Add a utility fn to remove dependency on drain_filter, which is still…
Jan 14, 2021
823f695
cargo fmt
Jan 14, 2021
cf48368
cargo fmt
Jan 15, 2021
a53e1ba
cargo clippy
Jan 15, 2021
fce6354
Change activations to return an iterator over all satisfying versions…
Jan 15, 2021
c4093f0
Move snapshot inline.
Jan 15, 2021
2c9c833
Changes from review and clippy.
Jan 19, 2021
5b241ab
Support crate for using directive, move parser and query-planner back…
Jan 13, 2021
d5f6ed9
Update members in Cargo.toml
Jan 27, 2021
01913b3
Merge qv/move-crates-to-top
Jan 27, 2021
f5dc04a
Borrow some cows and simplify the bootstrapping logic -- the first @c…
Jan 27, 2021
66343aa
Move activations into schema base file and make provide more generic.
Jan 27, 2021
ff31a5d
cargo fmt
Jan 27, 2021
f95f12a
Use bounds rather than checking first and last in tests.
Jan 27, 2021
3fb2974
Merge remote-tracking branch 'origin/main' into qv/using
abernix Feb 23, 2021
bc47795
Rename using to core-schema and change @core(using:) to @core(feature:)
Feb 25, 2021
46ce34a
Update failing tests.
Feb 25, 2021
549c523
Remove experimental, unsurfaced code coverage implementation.
abernix Feb 23, 2021
c6339a2
chore(fmt): run cargo fmt to resolve `graphql-parser` format inconsis…
abernix Feb 26, 2021
f0618bd
Merge branch 'main' into qv/using
abernix Feb 26, 2021
79e7d94
cargo fmt
abernix Feb 26, 2021
4cabe75
Merge branch 'main' into qv/using
abernix Feb 26, 2021
9dc6691
Merge branch 'main' into qv/using
abernix Mar 2, 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
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
[workspace]
members = [
"core-schema",
"graphql-parser",
"query-planner-wasm",
"query-planner",
"query-planner-wasm",
"stargate",
"stargate/crates/stargate-lib",
]
20 changes: 20 additions & 0 deletions core-schema/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
[package]
name = "core-schema"
version = "0.1.0"
authors = ["Ashi Krishnan <queerviolet@github.com>"]
edition = "2018"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
# workspace
graphql-parser = { path = "../graphql-parser" }

# 3rd party
lazy_static = "1.4.0"
regex = "1"
url = "2.2.0"
thiserror = "1.0.23"

[dev-dependencies]
insta = "1.5.2"
17 changes: 17 additions & 0 deletions core-schema/src/bounds.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/// Defines a `bounds` method returning the first and last
/// item from an Iterator.
pub trait Bounds: Iterator
where
Self::Item: Copy,
{
/// Get the first and last items from an iterator.
///
/// If the iterator has only one item, that item will
/// be returned as both the lower and upper bound.
fn bounds(&mut self) -> Option<(Self::Item, Self::Item)> {
let min = self.next();
min.map(move |min| (min, self.last().unwrap_or(min)))
}
}

impl<T: Copy, I: Iterator<Item = T>> Bounds for I {}
12 changes: 12 additions & 0 deletions core-schema/src/constants.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//! Constants, mainly supported versions of the `using` spec which we can
Copy link
Member

Choose a reason for hiding this comment

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

s/using/core/

//! reference during bootstrapping.

use std::borrow::Cow;

use crate::{spec::Spec, Version};

pub const CORE: Spec = Spec {
identity: Cow::Borrowed("https://lib.apollo.dev/core"),
name: Cow::Borrowed("core"),
version: Version(0, 1),
};
64 changes: 64 additions & 0 deletions core-schema/src/feature.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
//! A `Request` for a spec within a document.
//!
//! `Request`s are derived from `Directive`s during schema bootstrapping.
use std::borrow::Cow;

use graphql_parser::{
schema::{Directive, Value},
Pos,
};

use crate::spec::{Spec, SpecParseError};

/// Requests contain a `spec`, the `prefix` requested for that spec (which
Copy link
Member

Choose a reason for hiding this comment

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

Doc string here says prefix but field is actually name.

/// will be the spec's [default prefix](Spec.html#default_prefix) if none was
/// explicitly specified, and the position of the directive making the request
/// (for validation error reporting).
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct Feature {
pub spec: Spec,
pub name: Cow<'static, str>,
pub position: Pos,
}

impl Feature {
/// Extract a `Request` from a directive.
Copy link
Member

Choose a reason for hiding this comment

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

Request should be Feature?

///
/// This returns an `Option<Result<_, _>>`, which is admittedly odd! The reason
/// it does so is to represent two classes of extraction failures:
/// - If the directive *does not contain* a `spec` argument with a string value,
Copy link
Member

Choose a reason for hiding this comment

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

"spec" should be "feature" (many times in this comment)

/// this method returns `None`.
/// - If the directive *contains* a string `spec` argument, but that argument fails
/// to parse as a [`Spec`](Spec.html), this method returns `Some(Err(SpecParseError))`.
/// - If the directive contains a string `spec` argument which parses as a [`Spec`](Spec.html),
/// this method returns `Some(Ok(Request))`
///
/// This keeps `SpecParseError` from having to represent the "no spec argument at all" case,
/// which is impossible to reach from [`Spec::parse`](Spec.html#parse). It also simplifies
/// the bootstrapping code, which can simply use `filter_map` to collect `Result`s. (We track
/// `Result<Request, SpecParseError>` during bootstrapping to assist error reporting.)
pub(crate) fn from_directive(dir: &Directive) -> Option<Result<Feature, SpecParseError>> {
let mut spec: Option<Result<Spec, SpecParseError>> = None;
let mut prefix: Option<Cow<'static, str>> = None;
for (arg, val) in &dir.arguments {
if *arg == "feature" {
Copy link
Member

Choose a reason for hiding this comment

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

Are we not implementing the export argument?

if let Value::String(url) = val {
spec = Some(Spec::parse(url));
}
}
if *arg == "as" {
if let Value::String(prefix_str) = val {
Copy link
Member

Choose a reason for hiding this comment

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

Should we validate that the value is a valid prefix (has good characters)?

prefix = Some(Cow::Owned(prefix_str.clone()));
}
}
}

spec.map(|result| {
result.map(|spec| Feature {
name: prefix.unwrap_or_else(|| spec.name.clone()),
spec,
position: dir.position,
})
})
}
}
202 changes: 202 additions & 0 deletions core-schema/src/implementations.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
//! A set of spec implementations stored for easy lookup with
//! [`Schema.activations`](Schema.html#activations).

use std::{
borrow::Cow,
collections::{BTreeMap, HashMap},
};

use crate::{Feature, Version};

/// Implementations stores a set of implementations indexed by
/// spec identity and version.
pub struct Implementations<T>(HashMap<Cow<'static, str>, BTreeMap<Version, T>>);

impl<T> Implementations<T> {
pub fn new() -> Self {
Self(HashMap::new())
}

pub fn provide<Id, V>(mut self, identity: Id, version: V, implementation: T) -> Self
where
Id: Into<Cow<'static, str>>,
V: Into<Version>,
{
self.0
.entry(identity.into())
.or_default()
.entry(version.into())
.or_insert(implementation);
self
}

pub(crate) fn find<'a, S: AsRef<str>>(
&'a self,
identity: S,
version: &'a Version,
) -> Find<'a, T, impl Iterator<Item = Found<'a, T>>> {
let versions = self.0.get(identity.as_ref());
match versions {
Some(versions) => versions
.range(version..&Version(version.0, u64::MAX))
Copy link
Member

Choose a reason for hiding this comment

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

It's interesting (not necessary "wrong") that this code is sort of relying on knowing what the definition/implementation of satisfies but it still calls out to it as well. Honestly I suspect the number of elements of versions is never going to be particularly large (like, maybe 2 or 3 max?) and we can just drop this line to allow satisfies to encapsulate the satisfaction logic.

.filter(move |(impl_version, _)| impl_version.satisfies(version))
.into(),
None => Find::None,
}
}

pub fn find_feature<'a>(
&'a self,
feature: &'a Feature,
) -> Find<'a, T, impl Iterator<Item = Found<'a, T>>> {
self.find(&feature.spec.identity, &feature.spec.version)
}
}

pub type Found<'a, T> = (&'a Version, &'a T);
Copy link
Member

Choose a reason for hiding this comment

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

I can't say I really understand what's going on with the rest of the non-test part of the file. Is this just giving nicer names to something that's effectively Option<Iterator<(Version, T)>>?


pub enum Find<'a, T: 'a, I: Iterator<Item = Found<'a, T>>> {
None,
Found(I),
}

impl<'a, T, I> Iterator for Find<'a, T, I>
where
T: 'a,
I: Iterator<Item = Found<'a, T>>,
{
type Item = Found<'a, T>;

fn next(&mut self) -> Option<Self::Item> {
match self {
Self::None => None,
Self::Found(iter) => iter.next(),
}
}
}

impl<'a, T, I> From<I> for Find<'a, T, I>
where
T: 'a,
I: Iterator<Item = Found<'a, T>>,
{
fn from(iter: I) -> Self {
Self::Found(iter)
}
}

impl<T> Default for Implementations<T> {
fn default() -> Self {
Self::new()
}
}

#[cfg(test)]
mod tests {
use crate::{Bounds, Implementations, Version};

#[test]
fn it_finds_exact_matches() {
let identity = "https://spec.example.com/specA";
let impls = Implementations::new()
.provide(identity, Version(0, 9), "too small")
.provide(identity, Version(1, 0), "Specification A")
.provide(identity, Version(2, 0), "too big");

assert_eq!(
impls.find(&identity, &Version(1, 0)).collect::<Vec<_>>(),
vec![(&Version(1, 0), &"Specification A"),]
);

assert_eq!(
impls.find(&identity, &Version(1, 0)).bounds(),
Some((
(&Version(1, 0), &"Specification A"),
(&Version(1, 0), &"Specification A"),
))
);
}

#[test]
fn it_finds_satisfying_matches() {
let identity = "https://spec.example.com/specA";
let impls = Implementations::new()
.provide(identity, Version(0, 9), "too small")
.provide(identity, Version(2, 99), "2.99")
.provide(identity, Version(1, 0), "1.0")
.provide(identity, Version(1, 2), "1.2")
.provide(identity, Version(1, 3), "1.3")
.provide(identity, Version(1, 5), "1.5")
.provide(identity, Version(2, 0), "2.0");

assert_eq!(
impls.find(&identity, &Version(1, 0)).collect::<Vec<_>>(),
vec![
(&Version(1, 0), &"1.0"),
(&Version(1, 2), &"1.2"),
(&Version(1, 3), &"1.3"),
(&Version(1, 5), &"1.5"),
]
);

assert_eq!(
impls.find(&identity, &Version(1, 0)).bounds(),
Some(((&Version(1, 0), &"1.0"), (&Version(1, 5), &"1.5"),))
);

assert_eq!(
impls.find(&identity, &Version(2, 1)).collect::<Vec<_>>(),
vec![(&Version(2, 99), &"2.99"),]
);
}

#[test]
fn it_ignores_unrelated_specs() {
let identity = "https://spec.example.com/specA";
let unrelated = "https://spec.example.com/B";
let impls = Implementations::new()
.provide(identity, Version(0, 9), "too small")
.provide(identity, Version(2, 99), "2.99")
.provide(unrelated, Version(1, 3), "unrelated 1.3")
.provide(identity, Version(1, 0), "1.0")
.provide(unrelated, Version(1, 2), "unrelated 1.2")
.provide(identity, Version(1, 2), "1.2")
.provide(unrelated, Version(1, 5), "unrelated 1.5")
.provide(identity, Version(1, 3), "1.3")
.provide(identity, Version(1, 5), "1.5")
.provide(unrelated, Version(2, 0), "2.0")
.provide(identity, Version(2, 0), "2.0");
assert_eq!(
impls.find(&identity, &Version(1, 0)).collect::<Vec<_>>(),
vec![
(&Version(1, 0), &"1.0"),
(&Version(1, 2), &"1.2"),
(&Version(1, 3), &"1.3"),
(&Version(1, 5), &"1.5"),
]
);
assert_eq!(
impls.find(&identity, &Version(2, 1)).next(),
Some((&Version(2, 99), &"2.99"))
);
}

#[test]
fn it_treats_each_zerodot_version_as_mutually_incompatible() {
let identity = "https://spec.example.com/specA";
let impls = Implementations::new()
.provide(identity, Version(0, 0), "0.0")
.provide(identity, Version(0, 1), "0.1")
.provide(identity, Version(0, 2), "0.0")
.provide(identity, Version(0, 3), "0.1")
.provide(identity, Version(0, 99), "0.99");
assert_eq!(
impls.find(&identity, &Version(0, 1)).bounds(),
Some(((&Version(0, 1), &"0.1"), (&Version(0, 1), &"0.1")))
);
assert_eq!(
impls.find(&identity, &Version(0, 99)).bounds(),
Some(((&Version(0, 99), &"0.99"), (&Version(0, 99), &"0.99")))
);
}
}
23 changes: 23 additions & 0 deletions core-schema/src/lib.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
mod version;
pub use version::*;

mod feature;
pub use feature::*;

mod spec;
pub use spec::*;

mod constants;
pub use constants::*;

mod schema;
pub use schema::*;

mod bounds;
pub use bounds::*;

mod implementations;
pub use implementations::*;

pub use graphql_parser::ParseError as GraphQLParseError;
pub use graphql_parser::Pos;
Loading