FuelLabs / fuel-indexer

*Archived* 🗃 The Fuel indexer is a standalone service that can be used to index various components of the Fuel blockchain.
140 stars 67 forks source link

enhancement: refactor GraphQL schema parser #1281

Closed lostman closed 1 year ago

lostman commented 1 year ago

Description

This PR refactors the ParsedGraphQLSchema::new() function and removes unnecessary content from the struct.

All parsing logic is moved to a separate struct, SchemaDecoder.

ServiceDocument and raw GraphQLSchema are removed from ParsedGraphQLSchema. ServiceDocument was only used in the code generator, and we only used the type definitions. We don't need both since we already have these in the parsed content—in type_defs. As for GraphQLSchema, we only need the version, not the raw schema String.

Testing steps

CI testing is sufficient.

Changelog

Please add neat Changelog info here, according to our Contributor's Guide.

ra0x3 commented 1 year ago

@lostman

lostman commented 1 year ago

@ra0x3, if we want to keep it as small as possible, we can remove the following:

ast: ServiceDocument

It is used in only one place:

    for definition in schema.ast().definitions.iter() {
        if let Some(def) = process_definition(&schema, definition) {
            output = quote! {
                #output
                #def
            };
        }
    }

And process_definition produces an output only for Types

    match definition {
        TypeSystemDefinition::Type(def) => process_type_def(schema, &def.node),
        TypeSystemDefinition::Schema(_def) => None,
        def => {
            panic!("Unhandled definition type: {def:?}");
        }
    }

If we want to throw any errors, like the above panic in the case of a Directive, we can do that at the parser level.

We could instead refer to type_defs directly:

    for (_, type_definition) in schema.type_defs().iter() {
        if let Some(def) = process_type_def(&schema, type_definition) {
        }
    }

Serialize/Deserialize

This poses a problem. The types we use from async-graphql-parser (e.g., TypeDefinition) are not serializable. We would have to write a serializer by hand.

Do we really need to serialize this struct? Loading the schema from the DB and parsing it shouldn't be a bottleneck. And for in-memory cache, we shouldn't need to serialize them.

One more thing, in addition to the ServiceDocument, we keep the raw schema String inside the ParsedGraphQL struct. It isn't used anywhere—only the version. So we should only keep the version and the parsed content. I did this in this PR, and I think it is the way to go, but I can always undo this change if needed.