hasura/graphql-engine
1
0
Fork 0
mirror of https://github.com/hasura/graphql-engine.git synced 2024-12-15 09:22:43 +03:00
Code Issues Projects Releases Wiki Activity

Use type rather than model names in JSONAPI sparse fields (#1349)

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

### What

Given a model name `Authors` with an `Author` type underneath, this
changes field selection for JSONAPI to use `fields[Author]` instead of
`fields[Authors]`.

This will make things more consistent when we are also filtering other
types from relationships or nested fields too, which will have to use
type names, for example
`fields[Author]=name,articles&fields[Article]=title`

V3_GIT_ORIGIN_REV_ID: fff43c673888877ca8b2edb6e5ca9fdc846675a5
This commit is contained in:
Daniel Harvey 2024-11-15 13:39:40 +00:00 committed by hasura-bot
parent 5c7e20f83b
commit bf61f3e18b
10 changed files with 52 additions and 57 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
v3/crates/jsonapi/src/parse.rs
View File

@ -5,7 +5,7 @@ use open_dds::{
identifier,
identifier::{Identifier, SubgraphName},
models::ModelName,
types::FieldName,
types::{CustomTypeName, FieldName},
};
use serde::{Deserialize, Serialize};
mod filter;
@ -38,7 +38,7 @@ pub fn create_query_ir(
// create the selection fields; include all fields of the model output type
let mut selection = IndexMap::new();
for field_name in model.type_fields.keys() {
if include_field(query_string, field_name, &model.name.name) {
if include_field(query_string, field_name, &model.data_type.name) {
let field_name_ident = Identifier::new(field_name.as_str())
.map_err(|e| RequestError::BadRequest(e.into()))?;
@ -118,23 +118,23 @@ fn validate_sparse_fields(
model: &Model,
query_string: &jsonapi_library::query::Query,
) -> Result<(), RequestError> {
let model_name_string = model.name.name.to_string();
let type_name_string = model.data_type.name.to_string();
if let Some(fields) = &query_string.fields {
for (model_name, model_fields) in fields {
if *model_name == model_name_string {
for models_field in model_fields {
for (type_name, type_fields) in fields {
if *type_name == type_name_string {
for type_field in type_fields {
let string_fields: Vec<_> =
model.type_fields.keys().map(ToString::to_string).collect();
if !string_fields.contains(models_field) {
if !string_fields.contains(type_field) {
return Err(RequestError::BadRequest(format!(
"Unknown field in sparse fields: {models_field}"
"Unknown field in sparse fields: {type_field}"
)));
}
}
} else {
return Err(RequestError::BadRequest(format!(
"Unknown model in sparse fields: {model_name}"
"Unknown type in sparse fields: {type_name}"
)));
}
}
@ -143,30 +143,25 @@ fn validate_sparse_fields(
}
// 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
// this does not consider subgraphs at the moment - we match on `CustomTypeName` not
// `Qualified<CustomTypeName>`.
// This means that the below field is ambiguous where `Authors` type 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:
// This will need to be solved when we make users give JSONAPI types explicit names
// like we do in GraphQL
//
// 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,
object_type_name: &CustomTypeName,
) -> 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() {
if let Some(object_fields) = fields.get(object_type_name.0.as_str()) {
for object_field in object_fields {
if object_field == field_name.as_str() {
return true;
}
}

2
v3/crates/jsonapi/src/schema/parameters.rs
View File

@ -60,7 +60,7 @@ pub fn fields_parameter(model: &Model) -> oas3::spec::Parameter {
}
oas3::spec::Parameter {
name: format!("fields[{}]", model.name.name),
name: format!("fields[{}]", model.data_type.name),
allow_empty_value: None,
allow_reserved: None,
content: None,

2
v3/crates/jsonapi/tests/failing/select_model/Authors.txt
View File

@ -1 +1 @@
fields[Authors]=author_id,first_name,last_name&page[offset]=0&page[limit]=1
fields[Author]=author_id,first_name,last_name&page[offset]=0&page[limit]=1

10
v3/crates/jsonapi/tests/jsonapi_golden_tests.rs
View File

@ -24,7 +24,7 @@ fn test_get_succeeding_requests() {
let TestRequest { query, model_name } = test_request_setup(path);
// always test in `default` subgraph for now
let path = format!("/default/{model_name}");
let request_path = format!("/default/{model_name}");
let http_context = execute::HttpContext {
client: reqwest::Client::new(),
@ -40,7 +40,7 @@ fn test_get_succeeding_requests() {
&jsonapi_catalog,
metadata.into(),
axum::http::method::Method::GET,
axum::http::uri::Uri::from_str(&path).unwrap(),
axum::http::uri::Uri::from_str(&request_path).unwrap(),
query,
)
.await;
@ -52,7 +52,7 @@ fn test_get_succeeding_requests() {
result
);
}
Err(e) => panic!("expected success, instead got {e}"),
Err(e) => panic!("expected success for {path:?}, instead got {e}"),
}
});
});
@ -75,7 +75,7 @@ fn test_get_failing_requests() {
let TestRequest { query, model_name } = test_request_setup(path);
// always test in `default` subgraph for now
let path = format!("/default/{model_name}");
let request_path = format!("/default/{model_name}");
let http_context = execute::HttpContext {
client: reqwest::Client::new(),
@ -91,7 +91,7 @@ fn test_get_failing_requests() {
&jsonapi_catalog,
metadata.into(),
axum::http::method::Method::GET,
axum::http::uri::Uri::from_str(&path).unwrap(),
axum::http::uri::Uri::from_str(&request_path).unwrap(),
query,
)
.await;

2
v3/crates/jsonapi/tests/passing/select_model/Authors.txt
View File

@ -1 +1 @@
fields[Authors]=first_name&page[offset]=1&page[limit]=4
fields[Author]=first_name&page[offset]=1&page[limit]=4

14
v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_admin.snap
View File

@ -122,7 +122,7 @@ expression: generated_openapi
},
"/v1/rest/default/Articles": {
"get": {
"summary": "Fetch article values",
"summary": "Fetch Article values",
"parameters": [
{
"name": "page[limit]",
@ -143,7 +143,7 @@ expression: generated_openapi
"example": "10"
},
{
"name": "fields[Articles]",
"name": "fields[Article]",
"in": "query",
"description": "Optional list of fields from Articles to include in response. If no fields are provided, all fields are returned",
"schema": {
@ -199,7 +199,7 @@ expression: generated_openapi
"properties": {
"_type": {
"enum": [
"default_article"
"default_Article"
]
},
"attributes": {
@ -336,7 +336,7 @@ expression: generated_openapi
},
"/v1/rest/default/Authors": {
"get": {
"summary": "Fetch author values",
"summary": "Fetch Author values",
"description": "top level Authors model description",
"parameters": [
{
@ -358,7 +358,7 @@ expression: generated_openapi
"example": "10"
},
{
"name": "fields[Authors]",
"name": "fields[Author]",
"in": "query",
"description": "Optional list of fields from Authors to include in response. If no fields are provided, all fields are returned",
"schema": {
@ -411,7 +411,7 @@ expression: generated_openapi
"properties": {
"_type": {
"enum": [
"default_author"
"default_Author"
]
},
"attributes": {
@ -1644,7 +1644,7 @@ expression: generated_openapi
"example": "10"
},
{
"name": "fields[institutions]",
"name": "fields[institution]",
"in": "query",
"description": "Optional list of fields from institutions to include in response. If no fields are provided, all fields are returned",
"schema": {

6
v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_1.snap
View File

@ -116,7 +116,7 @@ expression: generated_openapi
},
"/v1/rest/default/Articles": {
"get": {
"summary": "Fetch article values",
"summary": "Fetch Article values",
"parameters": [
{
"name": "page[limit]",
@ -137,7 +137,7 @@ expression: generated_openapi
"example": "10"
},
{
"name": "fields[Articles]",
"name": "fields[Article]",
"in": "query",
"description": "Optional list of fields from Articles to include in response. If no fields are provided, all fields are returned",
"schema": {
@ -190,7 +190,7 @@ expression: generated_openapi
"properties": {
"_type": {
"enum": [
"default_article"
"default_Article"
]
},
"attributes": {

6
v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__generated_openapi_for_role_user_2.snap
View File

@ -12,7 +12,7 @@ expression: generated_openapi
"paths": {
"/v1/rest/default/Articles": {
"get": {
"summary": "Fetch article values",
"summary": "Fetch Article values",
"parameters": [
{
"name": "page[limit]",
@ -33,7 +33,7 @@ expression: generated_openapi
"example": "10"
},
{
"name": "fields[Articles]",
"name": "fields[Article]",
"in": "query",
"description": "Optional list of fields from Articles to include in response. If no fields are provided, all fields are returned",
"schema": {
@ -86,7 +86,7 @@ expression: generated_openapi
"properties": {
"_type": {
"enum": [
"default_article"
"default_Article"
]
},
"attributes": {

2
v3/crates/jsonapi/tests/snapshots/jsonapi_golden_tests__result_for_role_admin@select_model__Authors.txt.snap
View File

@ -8,7 +8,7 @@ DocumentData {
Multiple(
[
Resource {
_type: "default_author",
_type: "default_Author",
id: "1",
attributes: {
"first_name": String("John"),

24
v3/crates/jsonapi/tests/static/metadata.json
View File

@ -2057,7 +2057,7 @@
"kind": "ObjectType",
"version": "v1",
"definition": {
"name": "article",
"name": "Article",
"fields": [
{
"name": "article_id",
@ -2108,7 +2108,7 @@
"name": "article_bool_exp",
"operand": {
"object": {
"type": "article",
"type": "Article",
"comparableFields": [
{
"fieldName": "author_id",
@ -2141,7 +2141,7 @@
"kind": "TypePermissions",
"version": "v1",
"definition": {
"typeName": "article",
"typeName": "Article",
"permissions": [
{
"role": "admin",
@ -2168,7 +2168,7 @@
"kind": "ObjectType",
"version": "v1",
"definition": {
"name": "author",
"name": "Author",
"description": "author description",
"fields": [
{
@ -2221,7 +2221,7 @@
"name": "author_bool_exp",
"operand": {
"object": {
"type": "author",
"type": "Author",
"comparableFields": [
{
"fieldName": "author_id",
@ -2254,7 +2254,7 @@
"kind": "TypePermissions",
"version": "v1",
"definition": {
"typeName": "author",
"typeName": "Author",
"permissions": [
{
"role": "admin",
@ -2270,7 +2270,7 @@
"version": "v1",
"definition": {
"name": "Articles",
"objectType": "article",
"objectType": "Article",
"globalIdSource": true,
"source": {
"dataConnectorName": "db",
@ -2376,7 +2376,7 @@
"version": "v1",
"definition": {
"name": "Authors",
"objectType": "author",
"objectType": "Author",
"description": "top level Authors model description",
"globalIdSource": true,
"source": {
@ -5306,7 +5306,7 @@
},
{
"definition": {
"sourceType": "article",
"sourceType": "Article",
"name": "AuthorFromCommand",
"description": "AuthorFromCommand description",
"target": {
@ -5338,8 +5338,8 @@
"kind": "Relationship",
"version": "v1",
"definition": {
"sourceType": "author",
"name": "Articles",
"sourceType": "Author",
"name": "articles",
"target": {
"model": {
"name": "Articles",
@ -5370,7 +5370,7 @@
"kind": "Relationship",
"version": "v1",
"definition": {
"sourceType": "article",
"sourceType": "Article",
"name": "Author",
"target": {
"model": {