Skip to content

Commit

Permalink
Provide default empty object for empty model 'args' field (hasura#1154)
Browse files Browse the repository at this point in the history
<!-- The PR description should answer 2 important questions: -->

### What

If a model has arguments, but they are all provided by presets, then
previously we would require users to pass an empty `args: {}` argument
like this:

```graphql
query MyQuery
  ActorsByMovieMany(args: {}) {
    actor_id
    movie_id
    name
  }
}
```

There is no need for this, so this PR loosens this restriction, by
providing a default empty value. This means users can also do the above
query with:

```graphql
query MyQuery
  ActorsByMovieMany {
    actor_id
    movie_id
    name
  }
}
```

Because both versions now work, this is a non-breaking change.

### How

Instead of just looking at number of arguments in schema generation,
consider which have been prefilled and provide a default empty value if
there is nothing a user could pass anyway.

V3_GIT_ORIGIN_REV_ID: cf184e42a114df782e1480a8f19548dda31e5992
  • Loading branch information
danieljharvey authored and hasura-bot committed Sep 25, 2024
1 parent 601771c commit 0e93e22
Show file tree
Hide file tree
Showing 4 changed files with 91 additions and 6 deletions.
29 changes: 28 additions & 1 deletion v3/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,34 @@

### Changed

- OpenTelemetry service name set to `v3-engine` to avoid confusion with
- Making `args` non-compulsory for models where all arguments have presets.

Previously, if a model had arguments specified that were all provided by
presets, then we would require them to pass an empty `args: {}` argument:

```graphql
query MyQuery
ActorsByMovieMany(args: {}) {
actor_id
movie_id
name
}
}
```

This change loosens the restriction, so now the following query is valid too:

```graphql
query MyQuery
ActorsByMovieMany {
actor_id
movie_id
name
}
}
```

- OpenTelemetry service name set to `ddn-engine` to avoid confusion with
`graphql-engine`.

## [v2024.09.23]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,18 @@
"movie_id": 3,
"name": "Robert De Niro"
}
],
"noargs": [
{
"actor_id": 4,
"movie_id": 3,
"name": "Al Pacino"
},
{
"actor_id": 5,
"movie_id": 3,
"name": "Robert De Niro"
}
]
}
},
Expand All @@ -23,12 +35,20 @@
"movie_id": 4,
"name": "Morgan Freeman"
}
],
"noargs": [
{
"actor_id": 6,
"movie_id": 4,
"name": "Morgan Freeman"
}
]
}
},
{
"data": {
"ActorsByMovieMany": null
"ActorsByMovieMany": null,
"noargs": null
},
"errors": [
{
Expand All @@ -37,6 +57,13 @@
"extensions": {
"details": null
}
},
{
"message": "internal error: ndc_unexpected: ndc_client error: connector error: connector returned status code 400 Bad Request with message: missing argument movie_id",
"path": ["noargs"],
"extensions": {
"details": null
}
}
]
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,9 @@ query MyQuery {
movie_id
name
}
noargs: ActorsByMovieMany {
actor_id
movie_id
name
}
}
34 changes: 30 additions & 4 deletions v3/crates/graphql/schema/src/model_arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ use crate::{
GDS,
};
use lang_graphql::schema as gql_schema;
use metadata_resolve::Qualified;
use open_dds::models::ModelName;
use std::collections::{BTreeMap, HashMap};

use metadata_resolve::Qualified;

/// Creates the `args` input object within which the model
/// arguments fields will live.
pub fn get_model_arguments_input_field(
builder: &mut gql_schema::Builder<GDS>,
model: &metadata_resolve::ModelWithPermissions,
include_empty_default: bool,
) -> Result<gql_schema::InputField<GDS>, crate::Error> {
model
.graphql_api
Expand All @@ -34,6 +34,14 @@ pub fn get_model_arguments_input_field(
type_name: arguments_input_config.type_name.clone(),
});

// if there are no possible arguments, provide a default of `{}`
// so that `args` can be omitted if the user chooses
let default_value = if include_empty_default {
Some(lang_graphql::ast::value::ConstValue::Object(vec![]))
} else {
None
};

gql_schema::InputField {
name: arguments_input_config.field_name.clone(),
description: None,
Expand All @@ -43,7 +51,7 @@ pub fn get_model_arguments_input_field(
field_type: ast::TypeContainer::named_non_null(
arguments_input_config.type_name.clone(),
),
default_value: None,
default_value,
deprecation_status: gql_schema::DeprecationStatus::NotDeprecated,
}
})
Expand Down Expand Up @@ -138,8 +146,26 @@ pub fn add_model_arguments_field(
parent_field_name: &ast::Name,
parent_type: &ast::TypeName,
) -> Result<(), crate::Error> {
// which arguments are actually available for the user to provide?
let user_arguments: Vec<_> = model
.model
.arguments
.keys()
.filter(|argument_name| {
for permission in model.select_permissions.values() {
// if there is a preset for this argument, it will not be included in the schema
if permission.argument_presets.contains_key(*argument_name) {
return false;
}
}
true
})
.collect();

if !model.model.arguments.is_empty() {
let model_arguments_input = get_model_arguments_input_field(builder, model)?;
let include_empty_default = user_arguments.is_empty();
let model_arguments_input =
get_model_arguments_input_field(builder, model, include_empty_default)?;

let name = model_arguments_input.name.clone();

Expand Down

0 comments on commit 0e93e22

Please sign in to comment.