New GraphQL Filtering Design Pattern

[jeremypng]

I'm starting a discussion around re-designing the Strawberry Django/GraphQL filtering pattern in Netbox. I started out trying to solve for filtering by custom fields, but ran into several road blocks trying to infer all of this at import time. In addition, the current Strawberry implementation in Netbox has 12 open issues ( #18126, #18157, #18120, #18081, #17688, #17681, #16511, #16305, #16224, #12635, #11949, #7598 ) most of which are interrelated because the GraphQL Schema and filters are generated at import time with the autotype_decorator function which is actually introspecting the filtersets for each model. This works in some cases but not all because the filtersets are not how Strawberry implements filtering as pointed out by @arthanson in #17688.

I mostly agree with @arthanson here:

4. [Potential Best Option?] Write custom Strawberry filters for each of the filterset methods. This is what Strawberry is sort-of designed for, but it would require replicating / duplicating all the special NetBox filter handling in a Strawberry compatible way. Similar to 2 but leaves the existing filter code (non GraphQL) untouched.

pros: No changes to NetBox filter code. Can just be added, no POC needed. Less potential for edge-case bugs.
cons: Need to duplicate filtering logic specifically for GraphQL and plugins would need to do the same. Not DRY.

Note: Could create the Strawberry filters as Q objects as a first pass, then migrate those over to the filters code, thus doing item 2 above, but would allow time to test the filters in the real world.

I do agree that inferring the schema from the filtersets is pushing against the grain with Strawberry's design. However, I don't think that building proper Strawberry filters would necessarily be "replicating/duplicating all the special NetBox filter handling in a Strawberry compatible way."

The graphql system has an idiomatic way of crawling a data model that is not necessarily in line with the way the filtersets that currently feed the web front end and the DRF API. I think the GraphQL schema could be designed to interfere as little as possible with the data model allowing the user to just follow the data model relationships.

For instance, the Site model does not have an asn_id or asn field on it. However, the SiteFilterSet has asn_id and asn as a way to filter the asns relationship using a simple string match. This is built using introspection of the model's FilterSet by this code:

From filter_mixins.py:

declared_filters = filterset.declared_filters
        for fieldname, field in declared_filters.items():

            should_create_function, attr_type = map_strawberry_type(field)
            if attr_type is None:
                raise NotImplementedError(f"GraphQL Filter field unknown: {fieldname}: {field}")

            create_attribute_and_function(cls, fieldname, attr_type, should_create_function)

With this code you can filter sites in much the same way that you can via the DRF API:

query GetSites {
  site_list(filters: {asn_id: "1"}) {
    id
    name
    asns {
      id
      asn
    }
  }
}

The problem is that this results in filters that are only able to be processed if the Strawberry legacy filtering is left in place (or as in my attempts at this, you build custom resolvers and do all the query processing yourself which is brittle and guts Strawberry's power).

A Better Way

A more idiomatic approach would be to define the Strawberry SiteFilter with an asns attribute that is set to an ASNFilter type. Then you can filter like this:

query GetSites {
  site_list(filters: {asns: {id: "1"}) {
    id
    name
    asns {
      id
      asn
    }
  }
}

This allows you to just follow the datamodel. Basically, anywhere a model has a ForwardManyToOneDescriptor, ReverseManyToOneDescriptor, or ManyToManyDescriptor attached to it, we add an attribute to the Strawberry Filter that points to the filter for that model like this:

@strawberry_django.filter(models.Site, lookups=True)
class SiteFilter(ContactsFilterMixin, ImageAttachmentsFilterMixin, PrimaryModelFilterMixin):
    asns: Optional[ASNFilter] = strawberry.UNSET

Additionally, you can see new FilterMixin classes to inherit from to aid in building these Strawberry Filters.
Note: Models that have ReverseManyToOne foreign key fields would still have an _id field on the filter, because that is a field on the model (ie: tenant_id on Site).

Custom Fields

Finally, to deal with custom fields, we need to stay close to Strawberry's native implementation. Trying to build named filters on the custom field model filters not only requires introspection and a custom resolver (since these named filters, "cf_cust_id", don't actually exist as such on the model), but requires database access which is frowned upon at import time and also limits the ability to filter for newly created custom fields until a restart of Netbox.

The custom field data is stored in the custom_field_data field in the model and exposed via the CustomFieldsMixin:

@strawberry.type
class CustomFieldsMixin:

    @strawberry_django.field
    def custom_fields(self) -> strawberry.scalars.JSON:
        return self.custom_field_data

Strawberry_django.filters defines:

• BaseFilterLookup
• ComparisonFilterLookup
• DateFilterLookup
• DatetimeFilterLookup
• FilterLookup
• RangeLookup
• TimeFilterLookup

We just need to build a new FilterLookup method to allow filtering JSON data in the custom_fields attribute.

Extending our SiteFilter above with something like this:

@strawberry_django.filter(models.Site, lookups=True)
class SiteFilter(ContactsFilterMixin, ImageAttachmentsFilterMixin, PrimaryModelFilterMixin, JSONFilterMixin):
    asns: Optional[ASNFilter] = strawberry.UNSET
    custom_fields: Optional[JsonPathFilter] = None

This will ideally result in a stable schema that won't require Netbox restarts after custom fields are added/removed. I'm still working through the JSONFilterMixin and JSONPathFilter implementations, so I'm not sure if we'll be able to get away without a custom resolver yet.

Hopefully, this would give us a clear path to closing all 12 of the above issues with improved functionality.

I'd really like some feedback on this approach.

Thanks!
Jeremy Sanders

[jeremypng]

Update:

I have this new pattern working and have converted the Tenancy filters. I have built filter lookups, but none of this required any manual introspection at import time, the autotype_decorator, nor did it require any custom resolvers. All of the actual queries are performed by the native Strawberry-to-Django interface. The JSONFilter and TreeNode filters below are custom Filter Lookups that return a Q filter just like the native Strawberry Filter Lookups (ie: FilterLookup[str])

Related Model Filtering

• This is working well. You can filter using any of the attributes of the related model because we are just implementing the related model's filter

@strawberry_django.filter(models.Tenant, lookups=True)
class TenantFilter(PrimaryModelFilterMixin):
    name: FilterLookup[str] | None = strawberry_django.filter_field()
    slug: FilterLookup[str] | None = strawberry_django.filter_field()
    group: Annotated["TenantGroupFilter", strawberry.lazy('tenancy.graphql.filters')] | None = strawberry_django.filter_field()
    #more fields below

Custom field filtering

• is working with a JSON filter lookup that will query any JSON field (custom fields, config contexts, etc)

@dataclass
class CustomFieldsFilterMixin(BaseFilterMixin):
    custom_field_data: Annotated["JSONFilter", strawberry.lazy('core.graphql.filter_lookups')] | None = strawberry_django.filter_field()

TreeNode filtering

• I built another filter lookup for tree node searches for searching by models implementing Django MPTT (Region, Site Group, Location, ContactGroup, TenantGroup, WirelessLANGroup). Any model that is related to one of these MPTT models can use the TreeNode filter to find all objects (ie: Devices, Sites, Contacts, WirelessLANs, etc) that is a parent, child, sibling, ancestor, descendant, etc.

@strawberry_django.filter(models.Tenant, lookups=True)
class TenantFilter(PrimaryModelFilterMixin):
    name: FilterLookup[str] | None = strawberry_django.filter_field()
    slug: FilterLookup[str] | None = strawberry_django.filter_field()
    group: Annotated["TenantGroupFilter", strawberry.lazy('tenancy.graphql.filters')] | None = strawberry_django.filter_field()
    group_id: Annotated["TreeNodeFilter", strawberry.lazy('core.graphql.filter_lookups')] | None = strawberry_django.filter_field()
    #more fields below

[jeremypng]

Changes available for review here:
https://github.com/jeremypng/netbox/tree/refs/heads/graphql-filter-redesign

I commented on all of the related issues with fixed queries.

Fixed:
#17688, #11949, #7598, #18126, #17681, #16511, #16305

Strawberry Django bug that needs to be fixed: #18157

Further Investigation: #18120, the optimizer isn't kicking in here, which means it can't infer the relationships

No Fix due to non-bug: #18081

Requires db changes or housekeeper action: #12635

Pagination can be added if we get this change in: #16224

[jeremystretch]

Hi @jeremypng, apologies it's taken me a few days to respond.

First, I want to thank you for taking such initiative in addressing these issues. Few invest the time and effort to really dig in as you have, and I really appreciate it.

It's taken me some time to digest the current state of our GraphQL implementation in general (@arthanson has led work in that area recently), so please bear with me if I've missed anything. I haven't dug into custom fields or any of the more advanced filters yet, but want to clarify my understanding of your proposed implementation.

First, it's worth calling out explicitly that this approach sticks with Strawberry but moves us off of it's legacy filtering (disabling USE_DEPRECATED_FILTERS), which I think is the right path forward.

You've obviously spent a lot of time rewriting the the GraphQL type classes to explicitly declare each field (either locally or via a mixin). While this is valid, it tends to frustrate long-term maintenance, as we're duplicating to a degree the existing UI/REST filter sets. This is ideally avoided if possible, but potentially viable. I'm curious whether you explored an approach to generating these classes dynamically from their respective Django models, similar to what we're currently doing with filter sets.

Similarly, I see that you've created explicit Enums to represent choice sets. We'll definitely want to automate these as keeping them in sync would be burdensome, but that should be fairly easy for us to work out.

I'm going to keep digging into your proposal as time allows but wanted to let you know we're looking into this. Thanks again for all your work!

[jeremypng]

Thanks for taking a look.

You've obviously spent a lot of time rewriting the the GraphQL type classes to explicitly declare each field (either locally or via a mixin). While this is valid, it tends to frustrate long-term maintenance, as we're duplicating to a degree the existing UI/REST filter sets. This is ideally avoided if possible, but potentially viable. I'm curious whether you explored an approach to generating these classes dynamically from their respective Django models, similar to what we're currently doing with filter sets.

The problem with generating the filters dynamically is that they are self-referential. I tried a two pass approach where I annotated the filters w/ the autotype_decorator first, and then rebuilt the filters in the netbox/graphql/schema.py just before the schema was generated. This generated a valid schema, but then when you submit a query, the actual Strawberry view processor in netbox/graphql/views.py does not have those rebuilt filters available.

This is where I got to the end of that road on Saturday: new-graphql-pattern branch, just before I started this discussion:

• The main schema.py with the second pass at the filter build: schema.py
• The beefed up strawberry mapper and autotype_decorator: filter_mixins.py

In addition, because the filters were modified in the middle of importing, I had to use a custom resolver for the custom fields and to build the Q object which was turning into some pretty unfriendly code:

• Custom Resolver: resolver.py

Those are the reasons I abandoned the generated filters in favor of typing them out appropriately.

Using the late annotation and Strawberry.lazy() method allows the filters to reference one another without creating circular imports or trying to patch things in memory. Also, no custom resolvers are needed and the database query happens organically through the native Strawberry-Django path.

I'm not sure if you loaded up the GraphiQL interface, with the code running but the query interface is really a joy to use like this.

I haven't found any other Strawberry/Django examples doing the sort of introspection in the current autotype_decorator with django model/filtersets to derive the graphql filters either.

Similarly, I see that you've created explicit Enums to represent choice sets. We'll definitely want to automate these as keeping them in sync would be burdensome, but that should be fairly easy for us to work out.

Yes, I basically copied the choice files and ran some VI macros to generate the ENUMs. I think that could probably use a dynamic enum loader instead that would process the choice modules at import time.

Summary

I think the GraphQL and REST filtering interfaces serve different functions and are going to be incredibly hard to generate from one another. With this new approach, it's very easy to pinpoint a problem and fix it, as well as add custom functionality (like the parent IP Address and IP Range functions added here):

    @strawberry_django.filter_field()
    def parent(self, value: list[str], prefix) -> Q:
        if not value:
            return Q()
        q = Q()
        for subnet in value:
            try:
                query = str(netaddr.IPNetwork(subnet.strip()).cidr)
                q |= Q(address__net_host_contained=query)
            except (AddrFormatError, ValueError):
                return Q()
        return q

So, for the sanity and troubleshoot-ability that this creates, I think the slight duplication is worth it. With this approach, I was able to solve or point to the root issue of all of the open GraphQL issues some of which have been open for 3+ years.

I'm currently running this commit backported to 4.0.11 in our environment (as of this afternoon) and using the generated graphql schema to create strongly typed dynamic graphql queries using the GQL library tied to Pydantic models that define the query and filters against the Netbox GraphQL API. This simply wasn't possible w/ the loosely typed inferred approach.

Thanks again for taking a serious look at this!