Very basic sparse fieldset parsing (#1177)

<!-- The PR description should answer 2 important questions: -->

### What

A JSONAPI request allows users to provide the fields they require like
so:

```
fields[Authors]=author_id,first_name
```

This PR implements the most basic form of this, along with some notes on
what the better version could be. This is motivated by allowing us to
test the OpenDD pipeline more than anything else.

### How

Loop through sparse field sets looking for the field we want to include,
reject it if not.

JSONAPI is behind a feature flag so is a functional no-op.

V3_GIT_ORIGIN_REV_ID: 27dbb6103c42910be71529351e7a2c9ce0abcb4a
This commit is contained in:
Daniel Harvey 2024-09-30 10:58:04 +01:00 committed by hasura-bot
parent 18697ae98d
commit b470c927e1
9 changed files with 248 additions and 141 deletions

View File

@ -12,7 +12,7 @@ use open_dds::{
identifier,
identifier::{Identifier, SubgraphName},
models::ModelName,
types::CustomTypeName,
types::{CustomTypeName, FieldName},
};
use std::collections::{BTreeMap, HashMap};
use tracing_util::{ErrorVisibility, SpanVisibility, TraceableError, TraceableHttpResponse};
@ -69,7 +69,6 @@ pub async fn handler_internal<'metadata>(
Some(model) => {
// create the query IR
let query_ir = create_query_ir(model, &http_method, &uri, &query_string)?;
dbg!(&query_ir);
// execute the query with the query-engine
let result = query_engine_execute(&query_ir, metadata, &session, &http_context).await?;
// process result to JSON:API compliant response
@ -137,6 +136,40 @@ async fn resolve_ndc_query_execution<'ir>(
Ok(response.as_latest_rowsets())
}
// given the sparse fields for this request, should be include a given field in the query?
// this does not consider subgraphs at the moment - we match on `ModelName` not
// `Qualified<ModelName>`.
// This means that the below field is ambiguous where `Authors` model is defined in multiple
// subgraphs
// fields[Authors]=author_id,first_name
//
// two possible solutions:
// 1. make users qualify the name inline
//
// fields[subgraph.Authors]=author_id,first_name&fields[other.Authors]=author_id,last_name
//
// 2. much like we make users explicitly give GraphQL names to things, we
// make them give JSONAPI models an unambiguous name in metadata, and the user provides that:
//
// fields[subgraphAuthors]=author_id,firstName&fields[otherAuthors]=author_id,last_name
fn include_field(
query_string: &jsonapi_library::query::Query,
field_name: &FieldName,
model_name: &ModelName,
) -> bool {
if let Some(fields) = &query_string.fields {
if let Some(model_fields) = fields.get(model_name.as_str()) {
for model_field in model_fields {
if model_field == field_name.as_str() {
return true;
}
}
}
}
// if no sparse fields provided for our model, for now, default to including nothing
false
}
fn create_query_ir(
model: &ModelWithPermissions,
_http_method: &Method,
@ -152,21 +185,22 @@ fn create_query_ir(
} = parse_url(uri)?;
// create the selection fields; include all fields of the model output type
// TODO: parse 'sparse fieldsets' to include specific fields only
let mut selection = IndexMap::new();
for (field_name, _field_def) in &model.model.type_fields {
let field_name_ident = Identifier::new(field_name.as_str()).unwrap();
let field_name = open_dds::types::FieldName::new(field_name_ident.clone());
let field_alias = open_dds::query::Alias::new(field_name_ident);
let sub_sel =
open_dds::query::ObjectSubSelection::Field(open_dds::query::ObjectFieldSelection {
target: open_dds::query::ObjectFieldTarget {
arguments: IndexMap::new(),
field_name,
},
selection: None,
});
selection.insert(field_alias, sub_sel);
if include_field(query_string, field_name, &model.model.name.name) {
let field_name_ident = Identifier::new(field_name.as_str()).unwrap();
let field_name = open_dds::types::FieldName::new(field_name_ident.clone());
let field_alias = open_dds::query::Alias::new(field_name_ident);
let sub_sel =
open_dds::query::ObjectSubSelection::Field(open_dds::query::ObjectFieldSelection {
target: open_dds::query::ObjectFieldTarget {
arguments: IndexMap::new(),
field_name,
},
selection: None,
});
selection.insert(field_alias, sub_sel);
}
}
// create filters

View File

@ -2,62 +2,25 @@
use hasura_authn_core::{Identity, Role};
use std::collections::HashMap;
use std::path::PathBuf;
use std::path::{Path, PathBuf};
use std::str::FromStr;
use std::sync::Arc;
#[test]
fn test_get_requests() {
insta::glob!("fixtures/**/*.txt", |path| {
fn test_get_succeeding_requests() {
insta::glob!("passing/**/*.txt", |path| {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all() // this enables time and IO
.build()
.unwrap();
runtime.block_on(async {
let directory = path.parent().unwrap();
let model_name = path.file_stem().unwrap().to_str().unwrap();
let query_params = std::fs::read_to_string(path).unwrap_or_else(|error| {
panic!(
"{}: Could not read file {path:?}: {error}",
directory.display()
)
});
let jsonapi_query = jsonapi_library::query::Query::from_params(&query_params);
// Setup test context
let root_test_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests");
let metadata_path = root_test_dir.join("static").join("metadata.json");
let metadata_string =
std::fs::read_to_string(metadata_path.clone()).unwrap_or_else(|error| {
panic!(
"{}: Could not read file {path:?}: {error}",
metadata_path.display()
)
});
let metadata_json_value = serde_json::from_str(&metadata_string).unwrap();
let input_metadata: open_dds::Metadata = open_dds::traits::OpenDd::deserialize(
metadata_json_value,
jsonpath::JSONPath::new(),
)
.unwrap();
let configuration = get_metadata_resolve_configuration();
let (resolved_metadata, _) = metadata_resolve::resolve(input_metadata, &configuration)
.unwrap_or_else(|error| {
panic!(
"{}: Could not resolve metadata: {error}",
directory.display()
)
});
let jsonapi_state = jsonapi::State::new(&resolved_metadata);
let TestRequest {
query,
model_name,
jsonapi_state,
resolved_metadata,
} = test_setup(path);
// always test in `default` subgraph for now
let path = format!("/default/{model_name}");
@ -74,15 +37,118 @@ fn test_get_requests() {
&resolved_metadata,
axum::http::method::Method::GET,
axum::http::uri::Uri::from_str(&path).unwrap(),
jsonapi_query,
query,
)
.await;
insta::assert_debug_snapshot!("result", result);
match result {
Ok(result) => insta::assert_debug_snapshot!("result", result),
Err(e) => panic!("expected success, instead got {e}"),
}
});
});
}
#[test]
fn test_get_failing_requests() {
insta::glob!("failing/**/*.txt", |path| {
let runtime = tokio::runtime::Builder::new_current_thread()
.enable_all() // this enables time and IO
.build()
.unwrap();
runtime.block_on(async {
let TestRequest {
query,
model_name,
jsonapi_state,
resolved_metadata,
} = test_setup(path);
// always test in `default` subgraph for now
let path = format!("/default/{model_name}");
let http_context = execute::HttpContext {
client: reqwest::Client::new(),
ndc_response_size_limit: None,
};
let result = jsonapi::handler_internal(
Arc::new(http_context.clone()),
Arc::new(create_default_session()),
&jsonapi_state,
&resolved_metadata,
axum::http::method::Method::GET,
axum::http::uri::Uri::from_str(&path).unwrap(),
query,
)
.await;
match result {
Ok(_) => panic!("expected failure"),
Err(e) => insta::assert_debug_snapshot!("error", e),
}
});
});
}
struct TestRequest {
query: jsonapi_library::query::Query,
model_name: String,
jsonapi_state: jsonapi::State,
resolved_metadata: metadata_resolve::Metadata,
}
fn test_setup(path: &Path) -> TestRequest {
let directory = path.parent().unwrap();
let model_name = path.file_stem().unwrap().to_str().unwrap();
let query_params = std::fs::read_to_string(path).unwrap_or_else(|error| {
panic!(
"{}: Could not read file {path:?}: {error}",
directory.display()
)
});
let jsonapi_query = jsonapi_library::query::Query::from_params(&query_params);
// Setup test context
let root_test_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("tests");
let metadata_path = root_test_dir.join("static").join("metadata.json");
let metadata_string = std::fs::read_to_string(metadata_path.clone()).unwrap_or_else(|error| {
panic!(
"{}: Could not read file {path:?}: {error}",
metadata_path.display()
)
});
let metadata_json_value = serde_json::from_str(&metadata_string).unwrap();
let input_metadata: open_dds::Metadata =
open_dds::traits::OpenDd::deserialize(metadata_json_value, jsonpath::JSONPath::new())
.unwrap();
let configuration = get_metadata_resolve_configuration();
let (resolved_metadata, _) = metadata_resolve::resolve(input_metadata, &configuration)
.unwrap_or_else(|error| {
panic!(
"{}: Could not resolve metadata: {error}",
directory.display()
)
});
let jsonapi_state = jsonapi::State::new(&resolved_metadata);
TestRequest {
query: jsonapi_query,
model_name: model_name.to_string(),
jsonapi_state,
resolved_metadata,
}
}
// we will need to allow tests to define their own session options at some point
// the way that the GraphQL tests defined in `crates/engine/tests/execution.rs` do
fn create_default_session() -> hasura_authn_core::Session {

View File

@ -0,0 +1 @@
fields[Authors]=author_id,first_name&page[number]=3&page[size]=1

View File

@ -0,0 +1,10 @@
---
source: crates/jsonapi/tests/jsonapi_golden_tests.rs
expression: e
input_file: crates/jsonapi/tests/failing/select_model/Authors.txt
---
PlanError(
Permission(
"role admin does not have permission to select the field last_name from type author (in subgraph default) of model Authors (in subgraph default)",
),
)

View File

@ -1,74 +1,72 @@
---
source: crates/jsonapi/tests/jsonapi_golden_tests.rs
expression: result
input_file: crates/jsonapi/tests/fixtures/select_model/Artist.txt
input_file: crates/jsonapi/tests/passing/select_model/Artist.txt
---
Ok(
DocumentData {
data: Some(
Multiple(
[
Resource {
_type: "Artist (in subgraph default)",
id: "1",
attributes: {
"ArtistId": Number(1),
"Name": String("AC/DC"),
},
relationships: None,
links: None,
meta: None,
DocumentData {
data: Some(
Multiple(
[
Resource {
_type: "Artist (in subgraph default)",
id: "1",
attributes: {
"ArtistId": Number(1),
"Name": String("AC/DC"),
},
Resource {
_type: "Artist (in subgraph default)",
id: "2",
attributes: {
"ArtistId": Number(2),
"Name": String("Accept"),
},
relationships: None,
links: None,
meta: None,
relationships: None,
links: None,
meta: None,
},
Resource {
_type: "Artist (in subgraph default)",
id: "2",
attributes: {
"ArtistId": Number(2),
"Name": String("Accept"),
},
Resource {
_type: "Artist (in subgraph default)",
id: "3",
attributes: {
"ArtistId": Number(3),
"Name": String("Aerosmith"),
},
relationships: None,
links: None,
meta: None,
relationships: None,
links: None,
meta: None,
},
Resource {
_type: "Artist (in subgraph default)",
id: "3",
attributes: {
"ArtistId": Number(3),
"Name": String("Aerosmith"),
},
Resource {
_type: "Artist (in subgraph default)",
id: "4",
attributes: {
"ArtistId": Number(4),
"Name": String("Alanis Morissette"),
},
relationships: None,
links: None,
meta: None,
relationships: None,
links: None,
meta: None,
},
Resource {
_type: "Artist (in subgraph default)",
id: "4",
attributes: {
"ArtistId": Number(4),
"Name": String("Alanis Morissette"),
},
Resource {
_type: "Artist (in subgraph default)",
id: "5",
attributes: {
"ArtistId": Number(5),
"Name": String("Alice In Chains"),
},
relationships: None,
links: None,
meta: None,
relationships: None,
links: None,
meta: None,
},
Resource {
_type: "Artist (in subgraph default)",
id: "5",
attributes: {
"ArtistId": Number(5),
"Name": String("Alice In Chains"),
},
],
),
relationships: None,
links: None,
meta: None,
},
],
),
included: None,
links: None,
meta: None,
jsonapi: None,
},
)
),
included: None,
links: None,
meta: None,
jsonapi: None,
}

View File

@ -1,18 +1,16 @@
---
source: crates/jsonapi/tests/jsonapi_golden_tests.rs
expression: result
input_file: crates/jsonapi/tests/fixtures/select_model/Authors.txt
input_file: crates/jsonapi/tests/passing/select_model/Authors.txt
---
Ok(
DocumentData {
data: Some(
Multiple(
[],
),
DocumentData {
data: Some(
Multiple(
[],
),
included: None,
links: None,
meta: None,
jsonapi: None,
},
)
),
included: None,
links: None,
meta: None,
jsonapi: None,
}

View File

@ -2239,7 +2239,7 @@
{
"role": "admin",
"output": {
"allowedFields": ["author_id", "first_name", "last_name"]
"allowedFields": ["author_id", "first_name"]
}
}
]