Part 5 - Refactoring resolvers
This is part 5 of series
At the end of part 4 we have a working API that resolves topStory field. This is the setup:
# schema.py
@strawberry.type
class Query:
top_story: Story | None = strawberry.field(resolver=top_story.resolve)
# top_story.py
def resolve() -> Story | None:
first = next(node for node in db.nodes if node.get("__typename") == "Story")
if not first:
return None
return Story(
id=str(first.get("id")),
created_at=first.get("createdAt", ""),
title=first.get("title", ""),
summary=first.get("summary", ""),
updated_at=first.get("updated_at", ""),
like_count=first.get("like_count", ""),
does_viewer_like=first.get("does_viewer_like", ""),
category=Category[str(first.get("category"))],
attachments=[
Image.from_raw_data(attachment)
for attachment in first.get("attachments", [])
],
thumbnail=Image.from_raw_data(first.get("thumbnail")),
poster=actor.resolve(str(first.get("authorID"))),
)
This works, but I’m not happy with how this looks. There are 2 main issues with the current design:
Inconsistency
Both Story and Query are GraphQL types, but their fields are resolved using different approaches. Even within the Story type itself, fields are defined and resolved differently. For examples:
top_story
is defined using strawberry.field() with a resolvercreated_at
is defined by providing a typehint and let Strawberry automagically generates a GraphQL type.poster
is resolved by calling another resolver
Strawberry supports all of these, but these inconsistencies could add up overtime. I do not want to be in a situation where I need to make a decision on which approach to use everytime I add a new type or field. Inconsitency is always a sign of accidental complexity.
Eager fetching
When we resolve Story object, we always resolve poster
field regardless of whether client is asking for it. That could be a good option in some situations, for example when our a client always render post content and author info on one screen. For other situations, it’s wasteful.
Maybe turning poster into a function instead of a class property is a better option in general.
Giving these considerations, let’s refactor our code so that we use the same approach to define our field, plus each field is a evaluated lazily.
Outline:
- Move Query type to
types
folder just like other GraphQL types defined in our app - Replace each property with a method decorated by
@strawberry.field
. These methods are basically field resolvers. - Remove the entire
resolvers
folder - Rename the
id
property to have a prefix, e.g.story_id
andactor_id
. This is to avoid the confustion between the id defined in our code and the id generated by Strawberry. - Implement logic to resolve each field
- Import Query type to
schema.py
Commit: https://github.com/tuan/newsfeed-demo-app/commit/8eef5e604eb897d7d965314f8d6366622ff0b1b9
One thing to note in the refactoring is that our types, such as Story and Person, all take a “data” parameter. We could pass an “id” instead and let each type fetch data based on the ID. However, I’ve decided to let the “field” resolver do the data fetching and pass it down to each type instead. There are two reasons for this:
- It makes unit testing easier because you can easily mock the data when you construct the object.
- It provides flexibility in how the parent type handles fetching logic for each field. Imagine a situation, briefly mentioned earlier, where we choose eager fetching for all fields. We can do that by fetching all necessary data when the parent is resolved and passing down the corresponding slice for each field. When implementing each type, we could add support for both lazy and eager approaches
After making the changes, you can test the query again in GraphiQL to make sure every works:
There’s another test you could do is to log to console to see when poster
resolve logic is executed. Confirm that the log is printed to the console only when client explicitly makes request for poster field. This is how you set up logging to console in Django: https://github.com/tuan/newsfeed-demo-app/commit/7c46611aee8e5f90e5e632055c8dd9b8d3809e58