Add a way to filter fields

[patrick91]

This is a POC for allowing to filter/extend fields from the GraphQL schema, it came from this discord conversation: https://discord.com/channels/689806334337482765/1180117357943853116/1180117357943853116

I think it would be cool to allow filtering, but not sure if this is the nicest way (I don't like having to pass that function around), maybe we could think about using the schema config?

/cc @erikwrede @bellini666 @jkimbo for opinions 😊

[codecov]

Codecov Report

Merging #3274 (82b3898) into main (c26bb05) will decrease coverage by 0.03%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3274      +/-   ##
==========================================
- Coverage   96.62%   96.59%   -0.03%     
==========================================
  Files         485      486       +1     
  Lines       30292    30320      +28     
  Branches     3746     3749       +3     
==========================================
+ Hits        29270    29288      +18     
- Misses        833      839       +6     
- Partials      189      193       +4     

[codspeed-hq]

CodSpeed Performance Report

Merging #3274 will degrade performances by 21.72%

_{Comparing feature/visibility-filters-hooks (82b3898) with main (c26bb05)}

Summary

❌ 1 regressions
✅ 12 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	main	feature/visibility-filters-hooks	Change
❌	test_execute_basic	10 ms	12.8 ms	-21.72%

[jkimbo]

I like the idea. You might want to pass the parent type and the field name to the function as well to enable more advanced filtering.

[bellini666]

I like the idea of using the field's metadata. We probably should try to explore that more! :)

I think it would be cool to allow filtering, but not sure if this is the nicest way (I don't like having to pass that function around), maybe we could think about using the schema config?

I think a schema config for that doesn't exclude the possibility of having that get_fields, as it also enables the user to do some extra customization.

Another option (not sure how good it is) for this specifically would be to have a FieldExtension for that, like what @erikwrede was doing for the PermissionExtension. That extension can receive a is_visible_callback (or a better name) which gets called to check if that field should be included in the schema or not.

[skilkis]

Great idea! Defining filters / overrides I think is a great way to make Schema generation more customizable. See this comment here for a somewhat related aspect on when to apply FieldExtensions [1]. I like @bellini666's remark on using the field metadata, perhaps we could have multiple levels of filtering (Schema level, field level) where field level takes precedence over global filtering to allow overriding global rules on a per field basis.

[erikwrede]

This is a great PoC on schema filtering. Ultimately, I believe the final filtering should not happen inside the schema generator itself, but rather in some extension, following @bellini666's hint on FieldExtensions. Dynamically building schemas from the same type source is not an approach I'd recommend, there should rather be a possibility to dynamically filter the schemas for your different consumers. Everything else introduces additional complexity to maintaining the system. However, I see the use case for this and believe we should enable schema filtering in some way or the other.

[patrick91]

This is a great PoC on schema filtering. Ultimately, I believe the final filtering should not happen inside the schema generator itself, but rather in some extension, following @bellini666's hint on FieldExtensions.

How do you see that working?

Dynamically building schemas from the same type source is not an approach I'd recommend, there
should rather be a possibility to dynamically filter the schemas for your different consumers.

I'm not sure about the difference about the two approaches?

[erikwrede]

Dynamically building schemas from the same type source is not an approach I'd recommend, there
should rather be a possibility to dynamically filter the schemas for your different consumers.

I'm not sure about the difference about the two approaches?

The key difference is that you are operating on the schema and the same API endpoint, but introspection and access to types and fields depends on your role during access of the schema.

[patrick91]

Dynamically building schemas from the same type source is not an approach I'd recommend, there
should rather be a possibility to dynamically filter the schemas for your different consumers.

I'm not sure about the difference about the two approaches?

The key difference is that you are operating on the schema and the same API endpoint, but introspection and access to types and fields depends on your role during access of the schema.

Right, but that's more difficult to implement right, no? We either need to create the schema dynamically per request (which could be expensive, but maybe we can cache it), or we need to customise the introspection to not show these fields, plus also prevent them from being called :)

[jonfinerty]

Hi there, I'm original requester of this functionality in the discord thread, so I thought I'd share some of our use-case context, particularly around the dynamic building versus filtering.

We're attempting to show a subset of our API/types to external integrators, with just the data we're happy to show them (and so generally have a smaller integration surface area). We then broadly extend those graphql types for our own front-end use so we don't have to maintain duplicate python classes. In-fact, we actually have another superset schema on top of that of data/mutations we show/use in internal admin tooling.

Being able to compose an entirely different schema feels quite useful because we can (and I think prefer to) expose it on a different endpoint - in the case of the internal admin tooling we limit access to that endpoint to only our own tooling. The schema itself being more static (it's likely 80% of our schema for example will appear/disappear based on this) feels like it would play with other tooling a bit more neatly. For example we use strawberry export-schema a fair bit to do static type checking and codegen in our frontend, and other integrations.

Hopefully that's useful context, and do I appreciate it's our specific use-case you're trying to adapt to the general case.

[patrick91]

@jonfinerty thanks for the context!

I'll add another reference for inspiration: https://graphql-ruby.org/authorization/visibility.html

I'll probably take a day off next week to think about this more 😊

[erikwrede]

@patrick91 great source from the ruby docs, this one is more aligned with my suggestion of modifying the field visibility instead of managing two separate schemas. You would still be able to host your schema on separate endpoints, just injecting some endpoint information into the visibility-determining extension.

I prefer this approach for several reasons: first, it leaves schema generation untouched. Secondly, and most importantly, we avoid having future issues asking for swapping out certain types in some of the different sub-schema. This is an antipattern by design and we should avoid enabling users to build towards this kind of scenario.

I think the main concept we need to extract from this "user story" is the ability to modify the visibility of your schema for different audiences. And to achieve this, my favored approach is to have a single schema underneath that can be re-used, similar to what is described in the ruby docs.

Of course it's more expensive, so I'm opening to discuss.

[patrick91]

@patrick91 great source from the ruby docs, this one is more aligned with my suggestion of modifying the field visibility instead of managing two separate schemas. You would still be able to host your schema on separate endpoints, just injecting some endpoint information into the visibility-determining extension.

Have you also seen Apollo Federation contracts?

To me dynamic schemas sounds harder to think about, because a request can change the schema (and people could get crazy with dynamic filters), I wonder if there's a middle ground.

The ruby library also allows types to define a visibility (which in turns hides any field using that type if it is hidden), do we also want to do that? (Also they do it for arguments)

Also this PR approach has a centralised approach, do we prefer to move the check on the fields/types? Or shall we do both?

I prefer this approach for several reasons: first, it leaves schema generation untouched.

Is this because we are going to hack our solution on top of GraphQL core? I'd think having a new instance of the schema (if we could do it fast enough) would be better, as it reduces the number of places we need to touch to make sure all works well 😊

Secondly, and most importantly, we avoid having future issues asking for swapping out certain types in some of the different sub-schema. This is an antipattern by design and we should avoid enabling users to build towards this kind of scenario.

Do you have an example of this?

[patrick91]

@erik I think I'm ok with merging this PR as is, it's not as flexible as it could be, but I think it unblocks @jonfinerty without us having to commit to a big API change/addition 😊

@jkimbo just noticed your comment 🤦‍♂️, we already pass the full object type so we should be able complex filters 😊

For the dynamic filtering we can chat here -> #3343

[patrick91]

Reminder for self: mention the hook in the docs before merging

[botberry]

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:

---

This release adds a new method get_fields on the Schema class.
You can use get_fields to hide certain field based on some conditions,
for example:

@strawberry.type
class User:
    name: str
    email: str = strawberry.field(metadata={"tags": ["internal"]})


@strawberry.type
class Query:
    user: User


def public_field_filter(field: StrawberryField) -> bool:
    return "internal" not in field.metadata.get("tags", [])


class PublicSchema(strawberry.Schema):
    def get_fields(
        self, type_definition: StrawberryObjectDefinition
    ) -> List[StrawberryField]:
        return list(filter(public_field_filter, type_definition.fields))


schema = PublicSchema(query=Query)

The schema here would only have the name field on the User type.

Here's the tweet text:

🆕 Release (next) is out! Thanks to @patrick91 for the PR 👏

Get it here 👉 https://beta.strawberry.rocks/release/(next)

[bellini666]

LGTM :)