Skip to content

Commit ef3dbfd

Browse files
fix(executor): project scalars with object values correctly (#492)
Ref ROUTER-159 - Handle object values within a scalar field like `JSON` correctly, previously it was just null - `Value::from` should check if it is integer first before float check because all integers are float while all floats are not integers; - To prevent serialization with extra `.0` in integers; ```diff - 42.0 + 42 ``` --------- Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
1 parent 704a112 commit ef3dbfd

File tree

2 files changed

+145
-13
lines changed

2 files changed

+145
-13
lines changed

lib/executor/src/projection/response.rs

Lines changed: 139 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,7 @@ pub fn project_by_operation(
8686
Ok(buffer)
8787
}
8888

89-
fn project_selection_set(
90-
data: &Value,
91-
errors: &mut Vec<GraphQLError>,
92-
selection: &FieldProjectionPlan,
93-
variable_values: &Option<HashMap<String, sonic_rs::Value>>,
94-
buffer: &mut Vec<u8>,
95-
) {
89+
fn project_without_selection_set(data: &Value, buffer: &mut Vec<u8>) {
9690
match data {
9791
Value::Null => buffer.put(NULL),
9892
Value::Bool(true) => buffer.put(TRUE),
@@ -101,6 +95,43 @@ fn project_selection_set(
10195
Value::I64(num) => write_i64(buffer, *num),
10296
Value::F64(num) => write_f64(buffer, *num),
10397
Value::String(value) => write_and_escape_string(buffer, value),
98+
Value::Object(value) => {
99+
buffer.put(OPEN_BRACE);
100+
let mut first = true;
101+
for (key, val) in value.iter() {
102+
if !first {
103+
buffer.put(COMMA);
104+
}
105+
write_and_escape_string(buffer, key);
106+
buffer.put(COLON);
107+
project_without_selection_set(val, buffer);
108+
first = false;
109+
}
110+
buffer.put(CLOSE_BRACE);
111+
}
112+
Value::Array(arr) => {
113+
buffer.put(OPEN_BRACKET);
114+
let mut first = true;
115+
for item in arr.iter() {
116+
if !first {
117+
buffer.put(COMMA);
118+
}
119+
project_without_selection_set(item, buffer);
120+
first = false;
121+
}
122+
buffer.put(CLOSE_BRACKET);
123+
}
124+
};
125+
}
126+
127+
fn project_selection_set(
128+
data: &Value,
129+
errors: &mut Vec<GraphQLError>,
130+
selection: &FieldProjectionPlan,
131+
variable_values: &Option<HashMap<String, sonic_rs::Value>>,
132+
buffer: &mut Vec<u8>,
133+
) {
134+
match data {
104135
Value::Array(arr) => {
105136
buffer.put(OPEN_BRACKET);
106137
let mut first = true;
@@ -139,11 +170,15 @@ fn project_selection_set(
139170
}
140171
}
141172
None => {
142-
// If the selection set is not projected, we should return null
143-
buffer.put(NULL)
173+
// If the selection has no sub-selections, we serialize the whole object
174+
project_without_selection_set(data, buffer);
144175
}
145176
}
146177
}
178+
_ => {
179+
// If the data is not an object or array, we serialize it directly
180+
project_without_selection_set(data, buffer);
181+
}
147182
};
148183
}
149184

@@ -350,3 +385,98 @@ fn check(
350385
}
351386
}
352387
}
388+
389+
#[cfg(test)]
390+
mod tests {
391+
use graphql_parser::query::Definition;
392+
use hive_router_query_planner::{
393+
ast::{document::NormalizedDocument, normalization::create_normalized_document},
394+
consumer_schema::ConsumerSchema,
395+
utils::parsing::parse_operation,
396+
};
397+
use sonic_rs::json;
398+
399+
use crate::{
400+
introspection::schema::SchemaWithMetadata,
401+
projection::{plan::FieldProjectionPlan, response::project_by_operation},
402+
response::value::Value,
403+
};
404+
405+
#[test]
406+
fn project_scalars_with_object_value() {
407+
let supergraph = hive_router_query_planner::utils::parsing::parse_schema(
408+
r#"
409+
type Query {
410+
metadatas: Metadata!
411+
}
412+
413+
scalar JSON
414+
415+
type Metadata {
416+
id: ID!
417+
timestamp: String!
418+
data: JSON
419+
}
420+
"#,
421+
);
422+
let consumer_schema = ConsumerSchema::new_from_supergraph(&supergraph);
423+
let schema_metadata = consumer_schema.schema_metadata();
424+
let mut operation = parse_operation(
425+
r#"
426+
query GetMetadata {
427+
metadatas {
428+
id
429+
data
430+
}
431+
}
432+
"#,
433+
);
434+
let operation_ast = operation
435+
.definitions
436+
.iter_mut()
437+
.find_map(|def| match def {
438+
Definition::Operation(op) => Some(op),
439+
_ => None,
440+
})
441+
.unwrap();
442+
let normalized_operation: NormalizedDocument =
443+
create_normalized_document(operation_ast.clone(), Some("GetMetadata"));
444+
let (operation_type_name, selections) =
445+
FieldProjectionPlan::from_operation(&normalized_operation.operation, &schema_metadata);
446+
let data_json = json!({
447+
"__typename": "Query",
448+
"metadatas": [
449+
{
450+
"__typename": "Metadata",
451+
"id": "meta1",
452+
"timestamp": "2024-01-01T00:00:00Z",
453+
"data": {
454+
"float": 41.5,
455+
"int": -42,
456+
"str": "value1",
457+
"unsigned": 123,
458+
}
459+
},
460+
{
461+
"__typename": "Metadata",
462+
"id": "meta2",
463+
"data": null
464+
}
465+
]
466+
});
467+
let data = Value::from(data_json.as_ref());
468+
let projection = project_by_operation(
469+
&data,
470+
vec![],
471+
&None,
472+
operation_type_name,
473+
&selections,
474+
&None,
475+
1000,
476+
);
477+
let projected_bytes = projection.unwrap();
478+
let projected_str = String::from_utf8(projected_bytes).unwrap();
479+
let expected_response = r#"{"data":{"metadatas":[{"id":"meta1","data":{"float":41.5,"int":-42,"str":"value1","unsigned":123}},{"id":"meta2","data":null}]}}"#;
480+
assert_eq!(projected_str, expected_response);
481+
}
482+
}

lib/executor/src/response/value.rs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,16 +142,18 @@ impl<'a> Value<'a> {
142142
ValueRef::Bool(b) => Value::Bool(b),
143143
ValueRef::String(s) => Value::String(s.into()),
144144
ValueRef::Number(num) => {
145-
if let Some(num) = num.as_f64() {
146-
return Value::F64(num);
145+
if let Some(num) = num.as_u64() {
146+
return Value::U64(num);
147147
}
148148

149149
if let Some(num) = num.as_i64() {
150150
return Value::I64(num);
151151
}
152152

153-
if let Some(num) = num.as_u64() {
154-
return Value::U64(num);
153+
// All integers are floats, so to prevent to add
154+
// extra dots for integer numbers, we check for f64 last.
155+
if let Some(num) = num.as_f64() {
156+
return Value::F64(num);
155157
}
156158

157159
Value::Null

0 commit comments

Comments
 (0)