Skip to content
Closed
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
139 changes: 139 additions & 0 deletions .changeset/nested-array.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
---
router: patch
executor: patch
---

Handle nested abstract items while projecting correctly;

For the following schema;

```graphql
interface IContent {
id: ID!
}

interface IElementContent {
id: ID!
}

type ContentAChild {
title: String!
}

type ContentA implements IContent & IElementContent {
id: ID!
contentChildren: [ContentAChild!]!
}

type ContentBChild {
title: String!
}

type ContentB implements IContent & IElementContent {
id: ID!
contentChildren: [ContentBChild!]!
}

type Query {
contentPage: ContentPage!
}

type ContentPage {
contentBody: [ContentContainer!]!
}

type ContentContainer {
id: ID!
section: IContent
}
```

```graphql
query {
contentPage {
contentBody {
section {
...ContentAData
...ContentBData
}
}
}
}

fragment ContentAData on ContentA {
contentChildren {
title
}
}

fragment ContentBData on ContentB {
contentChildren {
title
}
}
```

If a query like above is executed, the projection plan should be able to handle nested abstract types correctly.

For the following subgraph response, array items should be handled by their own `__typename` values individually;

```json
{
"__typename": "Query",
"contentPage": [
{
"__typename": "ContentPage",
"contentBody": [
{
"__typename": "ContentContainer",
"id": "container1",
"section": {
"__typename": "ContentA",
"contentChildren": []
}
},
{
"__typename": "ContentContainer",
"id": "container2",
"section": {
"__typename": "ContentB",
"contentChildren": [
{
"__typename": "ContentBChild",
"title": "contentBChild1"
}
]
}
}
]
}
]
}
```

On the other hand if parent types of those don't have `__typename`, we don't need to check the parent types while projecting nested abstract items. In this case, the data to be projected would be;

```json
{
"contentPage": {
"contentBody": [
{
"id": "container1",
"section": {
"__typename": "ContentA",
"id": "contentA1",
"contentChildren": []
}
},
{
"id": "container2",
"section": {
"__typename": "ContentB",
"id": "contentB1",
"contentChildren": null
}
}
]
}
}
```
1 change: 1 addition & 0 deletions Cargo.lock

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

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,4 @@ vrl = { version = "0.29.0", features = ["compiler", "parser", "value", "diagnost
regex-automata = "0.4.10"
bytes = "1.10.1"
ahash = "0.8.12"
indexmap = "2.10.0"
3 changes: 2 additions & 1 deletion bin/router/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,13 @@ reqwest-middleware = { workspace = true }
vrl = { workspace = true }
serde_json = { workspace = true }
regex-automata = { workspace = true }
tokio-util = { workspace = true }
indexmap = { workspace = true }

mimalloc = { version = "0.1.48", features = ["v3"] }
moka = { version = "0.12.10", features = ["future"] }
hive-console-sdk = "0.2.2"
ulid = "1.2.1"
tokio-util = "0.7.16"
cookie = "0.18.1"
arc-swap = "1.7.1"
lasso2 = "0.8.2"
Expand Down
3 changes: 2 additions & 1 deletion bin/router/src/pipeline/authorization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ use hive_router_plan_executor::projection::plan::FieldProjectionPlan;
use hive_router_plan_executor::response::graphql_error::{GraphQLError, GraphQLErrorExtensions};
use hive_router_query_planner::ast::operation::OperationDefinition;

use indexmap::IndexMap;
pub use metadata::{
AuthorizationMetadata, AuthorizationMetadataError, ScopeId, ScopeInterner, UserAuthContext,
};
Expand All @@ -53,7 +54,7 @@ pub enum AuthorizationDecision {
/// The operation was modified to remove unauthorized parts. Continue with the new operation.
Modified {
new_operation_definition: OperationDefinition,
new_projection_plan: Vec<FieldProjectionPlan>,
new_projection_plan: IndexMap<String, FieldProjectionPlan>,
errors: Vec<AuthorizationError>,
},
/// The operation should be aborted due to unauthorized access and reject mode being enabled.
Expand Down
28 changes: 15 additions & 13 deletions bin/router/src/pipeline/authorization/rebuilder.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use std::{collections::HashSet, sync::Arc};
use std::collections::HashSet;

use hive_router_plan_executor::projection::plan::{FieldProjectionPlan, ProjectionValueSource};
use hive_router_query_planner::ast::{
operation::OperationDefinition, selection_item::SelectionItem, selection_set::SelectionSet,
value::Value,
};
use indexmap::IndexMap;

use crate::pipeline::authorization::tree::{PathIndex, UnauthorizedPathTrie};

Expand Down Expand Up @@ -121,9 +122,9 @@ fn rebuild_authorized_selection_set<'op>(

/// Rebuilds the projection plan to exclude unauthorized fields.
pub(super) fn rebuild_authorized_projection_plan(
original_plans: &Vec<FieldProjectionPlan>,
original_plans: &IndexMap<String, FieldProjectionPlan>,
unauthorized_path_trie: &UnauthorizedPathTrie,
) -> Vec<FieldProjectionPlan> {
) -> IndexMap<String, FieldProjectionPlan> {
rebuild_authorized_projection_plan_recursive(
original_plans,
unauthorized_path_trie,
Expand All @@ -134,23 +135,25 @@ pub(super) fn rebuild_authorized_projection_plan(

/// Recursively filters projection plans. Unauthorized fields become null.
fn rebuild_authorized_projection_plan_recursive(
original_plans: &Vec<FieldProjectionPlan>,
original_plans: &IndexMap<String, FieldProjectionPlan>,
unauthorized_path_trie: &UnauthorizedPathTrie,
path_position: PathIndex,
) -> Option<Vec<FieldProjectionPlan>> {
let mut authorized_plans = Vec::with_capacity(original_plans.len());
) -> Option<IndexMap<String, FieldProjectionPlan>> {
let mut authorized_plans = IndexMap::with_capacity(original_plans.len());

for plan in original_plans {
let path_segment = &plan.response_key;
for (path_segment, plan) in original_plans {
let Some((child_path_position, is_unauthorized)) =
unauthorized_path_trie.find_field(path_position, path_segment)
else {
authorized_plans.push(plan.clone());
authorized_plans.insert(path_segment.clone(), plan.clone());
continue;
};

if is_unauthorized {
authorized_plans.push(plan.with_new_value(ProjectionValueSource::Null));
authorized_plans.insert(
path_segment.clone(),
plan.with_new_value(ProjectionValueSource::Null),
);
continue;
}

Expand All @@ -162,12 +165,11 @@ fn rebuild_authorized_projection_plan_recursive(
selections,
unauthorized_path_trie,
child_path_position,
)
.map(Arc::new),
),
},
other => other.clone(),
};
authorized_plans.push(plan.with_new_value(new_value));
authorized_plans.insert(path_segment.clone(), plan.with_new_value(new_value));
}

if authorized_plans.is_empty() {
Expand Down
3 changes: 2 additions & 1 deletion bin/router/src/pipeline/normalize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use hive_router_plan_executor::introspection::partition::partition_operation;
use hive_router_plan_executor::projection::plan::FieldProjectionPlan;
use hive_router_query_planner::ast::normalization::normalize_operation;
use hive_router_query_planner::ast::operation::OperationDefinition;
use indexmap::IndexMap;
use ntex::web::HttpRequest;
use xxhash_rust::xxh3::Xxh3;

Expand All @@ -20,7 +21,7 @@ pub struct GraphQLNormalizationPayload {
pub operation_for_plan: Arc<OperationDefinition>,
pub operation_for_introspection: Option<Arc<OperationDefinition>>,
pub root_type_name: &'static str,
pub projection_plan: Arc<Vec<FieldProjectionPlan>>,
pub projection_plan: Arc<IndexMap<String, FieldProjectionPlan>>,
}

#[inline]
Expand Down
2 changes: 1 addition & 1 deletion lib/executor/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ hyper-util = { version = "0.1.16", features = [
bytes = { workspace = true }
itoa = "1.0.15"
ryu = "1.0.20"
indexmap = "2.10.0"
indexmap = { workspace = true }
bumpalo = "3.19.0"

[dev-dependencies]
Expand Down
3 changes: 2 additions & 1 deletion lib/executor/src/execution/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use hive_router_query_planner::planner::plan_nodes::{
QueryPlan, SequenceNode,
};
use http::HeaderMap;
use indexmap::IndexMap;
use serde::Deserialize;
use sonic_rs::ValueRef;

Expand Down Expand Up @@ -50,7 +51,7 @@ use crate::{

pub struct QueryPlanExecutionContext<'exec, 'req> {
pub query_plan: &'exec QueryPlan,
pub projection_plan: &'exec Vec<FieldProjectionPlan>,
pub projection_plan: &'exec IndexMap<String, FieldProjectionPlan>,
pub headers_plan: &'exec HeaderRulesPlan,
pub variable_values: &'exec Option<HashMap<String, sonic_rs::Value>>,
pub extensions: Option<HashMap<String, sonic_rs::Value>>,
Expand Down
Loading