Skip to content

fix(2892): consider entity resolvers in n + 1 check #2978

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

Merged
merged 27 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
e7dbb91
feat(npo): implement n+1 checks for entity resolvers
meskill Oct 8, 2024
351a90d
fix caching
meskill Oct 8, 2024
47b546a
fix lint
meskill Oct 8, 2024
5d2fbd7
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Oct 8, 2024
ec2bf54
update message for snaps
tusharmath Oct 9, 2024
051e259
Merge branch 'main' into fix/npo-entity
tusharmath Oct 9, 2024
2d4d2af
revert type resolver
tusharmath Oct 9, 2024
503288f
test: add additional test case
meskill Oct 14, 2024
5894dfc
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Oct 14, 2024
39f1770
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Oct 14, 2024
bf6fd5f
fix test config
meskill Oct 17, 2024
4a8cb3f
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Oct 17, 2024
637c660
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Oct 21, 2024
436d906
fix lint
meskill Oct 21, 2024
6305b93
Merge branch 'main' into fix/npo-entity
tusharmath Oct 23, 2024
9fe4750
Merge branch 'main' into fix/npo-entity
meskill Oct 29, 2024
7bd5e65
fix fixtures
meskill Oct 29, 2024
e5b03c8
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Nov 7, 2024
e04d649
simplify the code and add more tests
meskill Nov 17, 2024
f15f2cb
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Nov 21, 2024
96e6bf1
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Nov 26, 2024
5bf0761
fix chunk usage
meskill Nov 26, 2024
0baa0e8
Merge branch 'main' into fix/npo-entity
meskill Nov 27, 2024
ad9051a
Merge branch 'main' into fix/npo-entity
meskill Nov 27, 2024
f667e8e
Merge branch 'main' into fix/npo-entity
amitksingh1490 Nov 28, 2024
71915a6
Merge remote-tracking branch 'origin/main' into fix/npo-entity
meskill Nov 28, 2024
bceaa40
fix compilation error
meskill Nov 28, 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
8 changes: 4 additions & 4 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ strum = "0.26.2"
tailcall-valid = { workspace = true }
dashmap = "6.1.0"
urlencoding = "2.1.3"
tailcall-chunk = "0.2.5"
tailcall-chunk = "0.3.0"

# to build rquickjs bindings on systems without builtin bindings
[target.'cfg(all(target_os = "windows", target_arch = "x86"))'.dependencies]
Expand Down
18 changes: 11 additions & 7 deletions src/core/config/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ use super::directive::Directive;
use super::from_document::from_document;
use super::{
AddField, Alias, Cache, Call, Discriminate, Expr, GraphQL, Grpc, Http, Link, Modify, Omit,
Protected, Resolver, ResolverSet, Server, Telemetry, Upstream, JS,
Protected, ResolverSet, Server, Telemetry, Upstream, JS,
};
use crate::core::config::npo::QueryPath;
use crate::core::config::source::Source;
Expand Down Expand Up @@ -136,6 +136,14 @@ impl Display for Type {
}

impl Type {
pub fn has_resolver(&self) -> bool {
self.resolvers.has_resolver()
}

pub fn has_batched_resolver(&self) -> bool {
self.resolvers.is_batched()
}

pub fn fields(mut self, fields: Vec<(&str, Field)>) -> Self {
let mut graphql_fields = BTreeMap::new();
for (name, field) in fields {
Expand Down Expand Up @@ -243,15 +251,11 @@ impl MergeRight for Field {

impl Field {
pub fn has_resolver(&self) -> bool {
!self.resolvers.is_empty()
self.resolvers.has_resolver()
}

pub fn has_batched_resolver(&self) -> bool {
if self.resolvers.is_empty() {
false
} else {
self.resolvers.iter().all(Resolver::is_batched)
}
self.resolvers.is_batched()
}

pub fn int() -> Self {
Expand Down
47 changes: 47 additions & 0 deletions src/core/config/npo/fixtures/entity-resolver.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
schema @server @upstream {
query: Query
}

type T1 @http(url: "") {
t1: Int
}

type T2 @http(url: "") {
t2: [N] @http(url: "")
}

type T3 @http(url: "", batchKey: ["id"]) {
t3: [N] @http(url: "")
}

type T4 @http(url: "", batchKey: ["id"]) {
t4: [N] @http(url: "", batchKey: ["id"])
}

type T5 @http(url: "", batchKey: ["id"]) {
t5: [String] @http(url: "", batchKey: ["id"])
}

type T6 @http(url: "", batchKey: ["id"]) {
t6: [Int]
}

type N {
n: [Int] @http(url: "")
}

type Query {
x: String
t1: T1 @http(url: "")
t2: T2 @http(url: "")
t3: T3 @http(url: "")
t4: T4 @http(url: "")
t5: T5 @http(url: "")
t6: T6 @http(url: "")
t1_ls: [T1] @http(url: "")
t2_ls: [T2] @http(url: "")
t3_ls: [T3] @http(url: "")
t4_ls: [T4] @http(url: "")
t5_ls: [T5] @http(url: "")
t6_ls: [T6] @http(url: "")
}
23 changes: 23 additions & 0 deletions src/core/config/npo/fixtures/multiple-deeply-nested.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
schema @server @upstream {
query: Query
}

type Root {
nested: Nested1
nested_list: [Nested1]
}

type Nested1 {
a: DeepNested @http(url: "")
b: DeepNested @http(url: "", batchKey: ["id"])
c: DeepNested @http(url: "")
d: [DeepNested] @http(url: "")
}

type DeepNested {
test: [String] @http(url: "")
}

type Query {
root: Root @http(url: "")
}
29 changes: 29 additions & 0 deletions src/core/config/npo/fixtures/multiple-type-usage.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
schema @server @upstream {
query: Query
}

type T1 {
t1: Int
}

type T2 {
t2: [N] @http(url: "")
}

type T3 {
t3: [N] @http(url: "", batchKey: ["id"])
}

type N {
n: Int @http(url: "")
}

type Query {
x: String
t1: T1 @http(url: "")
t2: T2 @http(url: "")
t3: T3 @http(url: "")
t1_ls: [T1] @http(url: "")
t2_ls: [T2] @http(url: "")
t3_ls: [T3] @http(url: "")
}
21 changes: 21 additions & 0 deletions src/core/config/npo/fixtures/nested.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
schema @server(port: 8030) {
query: Query
}

type Query {
version: Int! @expr(body: 1)
foo: [Foo!]! @expr(body: [{fizz: "buzz"}])
bar: [Foo!]! @expr(body: [{fizz: "buzz"}])
buzz: [Foo!]! @expr(body: [{fizz: "buzz"}])
}

type Foo {
fizz: String!
c: Int
}

type Foo {
fizz: String!
c: Int
spam: [String!]! @http(url: "https://example.com/spam", query: [{key: "id", value: "{{.value.fizz}}"}])
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@
source: src/core/config/npo/tracker.rs
expression: actual
---
query { f1 { f1 { f2 } } }
query { f1 { f2 } }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
source: src/core/config/npo/tracker.rs
expression: actual
snapshot_kind: text
---
query { t2 { t2 { n } } }
query { t2_ls { t2 } }
query { t3 { t3 { n } } }
query { t3_ls { t3 } }
query { t4 { t4 { n } } }
query { t4_ls { t4 { n } } }
query { __entities(representations: [{ __typename: "T1"}]) }
query { __entities(representations: [{ __typename: "T2"}]) }
query { __entities(representations: [{ __typename: "T3"}]) { t3 } }
query { __entities(representations: [{ __typename: "T4"}]) { t4 { n } } }
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
source: src/core/config/npo/tracker.rs
expression: actual
snapshot_kind: text
---
query { root { nested { d { test } } } }
query { root { nested_list { a } } }
query { root { nested_list { b { test } } } }
query { root { nested_list { c } } }
query { root { nested_list { d } } }
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
source: src/core/config/npo/tracker.rs
expression: actual
snapshot_kind: text
---
query { f1 { f2 } }
query { f1 { f3 } }
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
source: src/core/config/npo/tracker.rs
expression: actual
---
query { t2 { t2 { n } } }
query { t2_ls { t2 } }
query { t3 { t3 { n } } }
query { t3_ls { t3 { n } } }
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
source: src/core/config/npo/tracker.rs
expression: actual
---
query { bar { spam } }
query { buzz { spam } }
query { foo { spam } }
Loading
Loading