Simplify namespaced AllowAll (#702)

<!-- Thank you for submitting this PR! :) -->

## Description

It turns out that every usage of `Builder::allow_all_namespaced`
concerned itself with either introspection nodes or (in the GDS case)
`None`.

Combined with the fact that both the SDL and GDS implementations of
`SchemaContext::introspection_namespace_node` were trivial, the
`NamespacedNodeInfo` contained in
`Namespaced::AllowAll(S::NamespacedNodeInfo)` is always functionally
meaningless.

This PR removes the `NamespacedNodeInfo` from `Namespaced::AllowAll` as
well as `SchemaContext::introspection_namespace_node`, thus simplifying
the implementation.

<!--
  Questions to consider answering:
  1. What user-facing changes are being made?
2. What are issues related to this PR? (Consider adding `(close
#<issue-no>)` to the PR title)
  3. What is the conceptual design behind this PR?
  4. How can this PR be tested/verified?
  5. Does the PR have limitations?
  6. Does the PR introduce breaking changes?
-->

## Changelog

- Add a changelog entry (in the "Changelog entry" section below) if the
changes
  in this PR have any user-facing impact. See
[changelog
guide](https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide).
- If no changelog is required ignore/remove this section and add a
  `no-changelog-required` label to the PR.

### Product

_(Select all products this will be available in)_

- [ ] community-edition
- [ ] cloud
<!-- product : end : DO NOT REMOVE -->

### Type

<!-- See changelog structure:
https://github.com/hasura/graphql-engine-mono/wiki/Changelog-Guide#structure-of-our-changelog
-->

_(Select only one. In case of multiple, choose the most appropriate)_

- [ ] highlight
- [ ] enhancement
- [ ] bugfix
- [ ] behaviour-change
- [ ] performance-enhancement
- [ ] security-fix
<!-- type : end : DO NOT REMOVE -->

### Changelog entry

<!--
  - Add a user understandable changelog entry
- Include all details needed to understand the change. Try including
links to docs or issues if relevant
  - For Highlights start with a H4 heading (#### <entry title>)
  - Get the changelog entry reviewed by your team
-->

_Replace with changelog entry_

<!-- changelog-entry : end : DO NOT REMOVE -->

<!-- changelog : end : DO NOT REMOVE -->

V3_GIT_ORIGIN_REV_ID: 1525905488203ff07182aa700c9fff26512743ab
This commit is contained in:
Philip Lykke Carlsen 2024-06-11 19:07:53 +02:00 committed by hasura-bot
parent a3ea579006
commit b3641238a9
14 changed files with 111 additions and 162 deletions

View File

@ -88,7 +88,6 @@ pub trait SchemaContext: std::fmt::Debug + Clone + PartialEq + Serialize {
// Option<&GenericNodeInfo>. We can then use None to store empty information about
// introspection fields.
fn introspection_node() -> Self::GenericNodeInfo;
fn introspection_namespace_node() -> Self::NamespacedNodeInfo;
// Types and functions related to schema generation.
@ -127,13 +126,9 @@ impl<S: SchemaContext> Builder<S> {
RegisteredTypeName(type_name)
}
pub fn allow_all_namespaced<C>(
&mut self,
data: C,
namespaced_node_info: S::NamespacedNodeInfo,
) -> Namespaced<S, C> {
pub fn allow_all_namespaced<C>(&mut self, data: C) -> Namespaced<S, C> {
Namespaced {
namespaced: NamespacedData::AllowAll(namespaced_node_info),
namespaced: NamespacedData::AllowAll,
data,
}
}
@ -165,7 +160,7 @@ pub struct Namespaced<S: SchemaContext, C> {
#[derive(Serialize, Deserialize, PartialEq, Debug, Clone)]
pub enum NamespacedData<S: SchemaContext> {
AllowAll(S::NamespacedNodeInfo),
AllowAll,
Conditional(HashMap<S::Namespace, S::NamespacedNodeInfo>),
}
@ -223,20 +218,17 @@ pub struct Object<S: SchemaContext> {
}
fn build_typename_field<S: SchemaContext>(builder: &mut Builder<S>) -> Namespaced<S, Field<S>> {
builder.allow_all_namespaced(
Field {
name: mk_name!("__typename"),
description: None,
info: S::introspection_node(),
field_type: ast::Type {
base: ast::BaseType::Named(TypeName(mk_name!("String"))),
nullable: false,
},
arguments: BTreeMap::new(),
deprecation_status: DeprecationStatus::NotDeprecated,
builder.allow_all_namespaced(Field {
name: mk_name!("__typename"),
description: None,
info: S::introspection_node(),
field_type: ast::Type {
base: ast::BaseType::Named(TypeName(mk_name!("String"))),
nullable: false,
},
S::introspection_namespace_node(),
)
arguments: BTreeMap::new(),
deprecation_status: DeprecationStatus::NotDeprecated,
})
}
impl<S: SchemaContext> Object<S> {

View File

@ -203,8 +203,7 @@ fn convert_enum_type_definition<S: SchemaContext>(
if values
.insert(
enum_value.clone(),
builder
.allow_all_namespaced(normalized_enum_value, S::introspection_namespace_node()),
builder.allow_all_namespaced(normalized_enum_value),
)
.is_some()
{}
@ -240,10 +239,7 @@ where
if arguments
.insert(
argument_name.clone(),
builder.allow_all_namespaced(
normalized_argument_definition,
S::introspection_namespace_node(),
),
builder.allow_all_namespaced(normalized_argument_definition),
)
.is_some()
{
@ -285,10 +281,7 @@ where
if fields
.insert(
field_name.clone(),
builder.allow_all_namespaced(
normalized_field_definition,
S::introspection_namespace_node(),
),
builder.allow_all_namespaced(normalized_field_definition),
)
.is_some()
{
@ -301,7 +294,7 @@ where
if implements
.insert(
register_type_name(builder, ast::TypeName(interface.item.clone())),
builder.allow_all_namespaced((), S::introspection_namespace_node()),
builder.allow_all_namespaced(()),
)
.is_some()
{
@ -338,10 +331,7 @@ where
if fields
.insert(
field_name.clone(),
builder.allow_all_namespaced(
normalized_field_definition,
S::introspection_namespace_node(),
),
builder.allow_all_namespaced(normalized_field_definition),
)
.is_some()
{
@ -354,7 +344,7 @@ where
if implements
.insert(
register_type_name(builder, ast::TypeName(interface.item.clone())),
builder.allow_all_namespaced((), S::introspection_namespace_node()),
builder.allow_all_namespaced(()),
)
.is_some()
{
@ -390,7 +380,7 @@ where
if members
.insert(
register_type_name(builder, ast::TypeName(member.item.clone())),
builder.allow_all_namespaced((), S::introspection_namespace_node()),
builder.allow_all_namespaced(()),
)
.is_some()
{
@ -458,10 +448,7 @@ where
if fields
.insert(
field_name.clone(),
builder.allow_all_namespaced(
normalized_field_definition,
S::introspection_namespace_node(),
),
builder.allow_all_namespaced(normalized_field_definition),
)
.is_some()
{

View File

@ -159,8 +159,6 @@ impl SchemaContext for SDL {
fn introspection_node() -> Self::GenericNodeInfo {}
fn introspection_namespace_node() -> Self::NamespacedNodeInfo {}
type TypeId = ast::Name;
fn to_type_name(type_id: &Self::TypeId) -> ast::TypeName {

View File

@ -57,8 +57,7 @@ pub fn apollo_federation_service_schema(
BTreeMap::new(),
gql_schema::DeprecationStatus::NotDeprecated,
);
let service_fields =
BTreeMap::from([(sdl_name, builder.allow_all_namespaced(sdl_field, None))]);
let service_fields = BTreeMap::from([(sdl_name, builder.allow_all_namespaced(sdl_field))]);
Ok(gql_schema::Object::new(
builder,
service_typename,

View File

@ -32,68 +32,59 @@ fn build_builtin_operator_schema(
input_fields.insert(
not_field_name.clone(),
builder.allow_all_namespaced(
gql_schema::InputField::<GDS>::new(
not_field_name.clone(),
None,
types::Annotation::Input(InputAnnotation::BooleanExpression(
BooleanExpressionAnnotation::BooleanExpressionArgument {
field: types::ModelFilterArgument::NotOp,
},
)),
ast::TypeContainer::named_null(gql_schema::RegisteredTypeName::new(
type_name.0.clone(),
)),
None,
gql_schema::DeprecationStatus::NotDeprecated,
),
builder.allow_all_namespaced(gql_schema::InputField::<GDS>::new(
not_field_name.clone(),
None,
),
types::Annotation::Input(InputAnnotation::BooleanExpression(
BooleanExpressionAnnotation::BooleanExpressionArgument {
field: types::ModelFilterArgument::NotOp,
},
)),
ast::TypeContainer::named_null(gql_schema::RegisteredTypeName::new(
type_name.0.clone(),
)),
None,
gql_schema::DeprecationStatus::NotDeprecated,
)),
);
let and_field_name = &boolean_expression_info.graphql_config.and_operator_name;
input_fields.insert(
and_field_name.clone(),
builder.allow_all_namespaced(
gql_schema::InputField::<GDS>::new(
and_field_name.clone(),
None,
types::Annotation::Input(InputAnnotation::BooleanExpression(
BooleanExpressionAnnotation::BooleanExpressionArgument {
field: types::ModelFilterArgument::AndOp,
},
)),
ast::TypeContainer::list_null(ast::TypeContainer::named_non_null(
gql_schema::RegisteredTypeName::new(type_name.0.clone()),
)),
None,
gql_schema::DeprecationStatus::NotDeprecated,
),
builder.allow_all_namespaced(gql_schema::InputField::<GDS>::new(
and_field_name.clone(),
None,
),
types::Annotation::Input(InputAnnotation::BooleanExpression(
BooleanExpressionAnnotation::BooleanExpressionArgument {
field: types::ModelFilterArgument::AndOp,
},
)),
ast::TypeContainer::list_null(ast::TypeContainer::named_non_null(
gql_schema::RegisteredTypeName::new(type_name.0.clone()),
)),
None,
gql_schema::DeprecationStatus::NotDeprecated,
)),
);
let or_field_name = &boolean_expression_info.graphql_config.or_operator_name;
input_fields.insert(
or_field_name.clone(),
builder.allow_all_namespaced(
gql_schema::InputField::<GDS>::new(
or_field_name.clone(),
None,
types::Annotation::Input(InputAnnotation::BooleanExpression(
BooleanExpressionAnnotation::BooleanExpressionArgument {
field: types::ModelFilterArgument::OrOp,
},
)),
ast::TypeContainer::list_null(ast::TypeContainer::named_non_null(
gql_schema::RegisteredTypeName::new(type_name.0.clone()),
)),
None,
gql_schema::DeprecationStatus::NotDeprecated,
),
builder.allow_all_namespaced(gql_schema::InputField::<GDS>::new(
or_field_name.clone(),
None,
),
types::Annotation::Input(InputAnnotation::BooleanExpression(
BooleanExpressionAnnotation::BooleanExpressionArgument {
field: types::ModelFilterArgument::OrOp,
},
)),
ast::TypeContainer::list_null(ast::TypeContainer::named_non_null(
gql_schema::RegisteredTypeName::new(type_name.0.clone()),
)),
None,
gql_schema::DeprecationStatus::NotDeprecated,
)),
);
input_fields

View File

@ -56,9 +56,7 @@ impl lang_graphql::schema::NamespacedGetter<GDS> for GDSRoleNamespaceGetter {
namespaced: &'s gql_schema::Namespaced<GDS, C>,
) -> Option<(&'s C, &'s <GDS as SchemaContext>::NamespacedNodeInfo)> {
match &namespaced.namespaced {
lang_graphql::schema::NamespacedData::AllowAll(namespaced_node_info) => {
Some((&namespaced.data, namespaced_node_info))
}
lang_graphql::schema::NamespacedData::AllowAll => Some((&namespaced.data, &None)),
lang_graphql::schema::NamespacedData::Conditional(map) => map
.get(&self.scope)
.map(|namespaced_node_info| (&namespaced.data, namespaced_node_info)),
@ -120,10 +118,6 @@ impl gql_schema::SchemaContext for GDS {
))
}
fn introspection_namespace_node() -> Self::NamespacedNodeInfo {
None
}
type TypeId = types::TypeId;
fn to_type_name(type_id: &Self::TypeId) -> ast::TypeName {

View File

@ -61,19 +61,16 @@ pub fn build_scalar_comparison_input(
input_fields.insert(
is_null_operator_name.clone(),
builder.allow_all_namespaced(
gql_schema::InputField::new(
is_null_operator_name.clone(),
None,
types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::IsNullOperation,
)),
is_null_input_type,
None,
gql_schema::DeprecationStatus::NotDeprecated,
),
builder.allow_all_namespaced(gql_schema::InputField::new(
is_null_operator_name.clone(),
None,
),
types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::IsNullOperation,
)),
is_null_input_type,
None,
gql_schema::DeprecationStatus::NotDeprecated,
)),
);
for (op_name, input_type) in operators {
@ -110,21 +107,18 @@ pub fn build_scalar_comparison_input(
input_fields.insert(
op_name.clone(),
builder.allow_all_namespaced(
gql_schema::InputField::new(
op_name.clone(),
None,
types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ComparisonOperation {
operator_mapping: this_operator_mapping,
},
)),
nullable_input_type,
None,
gql_schema::DeprecationStatus::NotDeprecated,
),
builder.allow_all_namespaced(gql_schema::InputField::new(
op_name.clone(),
None,
),
types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ComparisonOperation {
operator_mapping: this_operator_mapping,
},
)),
nullable_input_type,
None,
gql_schema::DeprecationStatus::NotDeprecated,
)),
);
}

View File

@ -34,37 +34,31 @@ pub fn build_order_by_enum_type_schema(
let asc_ast_name = &order_by_input_config.asc_direction_field_value;
order_by_values.insert(
asc_ast_name.clone(),
builder.allow_all_namespaced(
gql_schema::EnumValue {
value: asc_ast_name.clone(),
description: Some("Sorts the data in ascending order".to_string()),
deprecation_status: gql_schema::DeprecationStatus::NotDeprecated,
info: types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ModelOrderByDirection {
direction: types::ModelOrderByDirection::Asc,
},
)),
},
None,
),
builder.allow_all_namespaced(gql_schema::EnumValue {
value: asc_ast_name.clone(),
description: Some("Sorts the data in ascending order".to_string()),
deprecation_status: gql_schema::DeprecationStatus::NotDeprecated,
info: types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ModelOrderByDirection {
direction: types::ModelOrderByDirection::Asc,
},
)),
}),
);
let desc_ast_name = &order_by_input_config.desc_direction_field_value;
order_by_values.insert(
desc_ast_name.clone(),
builder.allow_all_namespaced(
gql_schema::EnumValue {
value: desc_ast_name.clone(),
description: Some("Sorts the data in descending order".to_string()),
deprecation_status: gql_schema::DeprecationStatus::NotDeprecated,
info: types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ModelOrderByDirection {
direction: types::ModelOrderByDirection::Desc,
},
)),
},
None,
),
builder.allow_all_namespaced(gql_schema::EnumValue {
value: desc_ast_name.clone(),
description: Some("Sorts the data in descending order".to_string()),
deprecation_status: gql_schema::DeprecationStatus::NotDeprecated,
info: types::Annotation::Input(types::InputAnnotation::Model(
types::ModelInputAnnotation::ModelOrderByDirection {
direction: types::ModelOrderByDirection::Desc,
},
)),
}),
);
Ok(gql_schema::TypeInfo::Enum(gql_schema::Enum {

View File

@ -113,7 +113,7 @@ pub fn query_root_schema(
if fields
.insert(
apollo_federation_service_field.name.clone(),
builder.allow_all_namespaced(apollo_federation_service_field.clone(), None),
builder.allow_all_namespaced(apollo_federation_service_field.clone()),
)
.is_some()
{

View File

@ -98,7 +98,7 @@ pub(crate) fn apollo_federation_field(
);
let entities_arguments = BTreeMap::from([(
mk_name!("representations"),
builder.allow_all_namespaced(representations_argument, None),
builder.allow_all_namespaced(representations_argument),
)]);
let entity_field = gql_schema::Field::new(
mk_name!("_entities"),

View File

@ -105,7 +105,7 @@ pub(crate) fn relay_node_field(
// If a role implements the `node` field, then
// it should also have access to the `id` argument,
// which is why we use `allow_all_namespaced` here.
builder.allow_all_namespaced(id_argument, None),
builder.allow_all_namespaced(id_argument),
);
let mut relay_node_field_permissions = HashMap::new();
for (role, role_type_permission) in roles_type_permissions {

View File

@ -37,7 +37,7 @@ pub(crate) fn generate_select_many_arguments(
)?;
arguments.insert(
limit_argument.name.clone(),
builder.allow_all_namespaced(limit_argument, None),
builder.allow_all_namespaced(limit_argument),
);
}
@ -52,7 +52,7 @@ pub(crate) fn generate_select_many_arguments(
arguments.insert(
offset_argument.name.clone(),
builder.allow_all_namespaced(offset_argument, None),
builder.allow_all_namespaced(offset_argument),
);
}
@ -68,7 +68,7 @@ pub(crate) fn generate_select_many_arguments(
arguments.insert(
order_by_argument.name.clone(),
builder.allow_all_namespaced(order_by_argument, None),
builder.allow_all_namespaced(order_by_argument),
);
}
@ -114,7 +114,7 @@ pub(crate) fn generate_select_many_arguments(
arguments.insert(
where_argument.name.clone(),
builder.allow_all_namespaced(where_argument, None),
builder.allow_all_namespaced(where_argument),
);
}

View File

@ -51,7 +51,7 @@ pub(crate) fn select_one_field(
arguments.insert(
argument.name.clone(),
builder.allow_all_namespaced(argument, None),
builder.allow_all_namespaced(argument),
);
}

View File

@ -149,7 +149,7 @@ fn input_object_type_input_fields(
let namespaced_input_field = {
// if no input permissions are defined, include the field for all roles
if object_type_representation.type_input_permissions.is_empty() {
builder.allow_all_namespaced(input_field, None)
builder.allow_all_namespaced(input_field)
// if input permissions are defined, include the field conditionally
} else {
let mut role_map = HashMap::new();