profusion / sgqlc

Simple GraphQL Client
https://sgqlc.readthedocs.io/
ISC License
513 stars 85 forks source link

Fragment Operation with inline and nested fragments "Unknown fragments..." error #211

Open aychang95 opened 2 years ago

aychang95 commented 2 years ago

First of all, I just want to thank the maintainers of sgqlc for making such a great and useful library like sgqlc. It works very well out of the box and provides creative convenience with the codegen tool!

We're relying on sgqlc heavily as a dependency in our new python SDK for Golden's Knowledge Graph Protocol here https://github.com/goldenrecursion/godel

So we've been running into an issue with our fragments not working correctly with the Operations generated by codegen.

Background

We have directories containing *.graphql files in fragments/ and queries/. Each file contains a single query/fragment. We run codegen to create a python module for each file. Queries import fragments and some fragments import other fragments as well.

Problem

We have a fragment that defined as:

fragment TripleWidget on Triple {
  ... on Statement {
    <nested fragments>
  }

The Operation generated from this fragment with codegen doesn't seem to work in general and/or with nested fragments:

from godel.fragments.EntityLink import fragment_entity_link
import sgqlc.types
import sgqlc.operation
import godel.schema

_schema = godel.schema
_schema_root = _schema.schema

__all__ = ('Operations',)

def fragment_triple_widget():
    _frag = sgqlc.operation.Fragment(_schema.Triple, 'TripleWidget')
    _frag__as__Statement = _frag.__as__(_schema.Statement)
    _frag__as__Statement.id()
    _frag__as__Statement.date_accepted()
    _frag__as__Statement.date_rejected()
    _frag__as__Statement.user_id()
    _frag__as__Statement.validation_status()
    _frag__as__Statement_subject = _frag__as__Statement.subject()
    _frag__as__Statement_subject.__fragment__(fragment_entity_link())
    _frag__as__Statement_predicate = _frag__as__Statement.predicate()
    _frag__as__Statement_predicate.id()
    _frag__as__Statement_predicate.name()
    _frag__as__Statement_predicate.description()
    _frag__as__Statement_predicate.label()
    _frag__as__Statement_predicate.object_type()
    _frag__as__Statement_predicate.show_in_infobox()
    _frag__as__Statement.object_value()
    _frag__as__Statement_object_entity = _frag__as__Statement.object_entity()
    _frag__as__Statement_object_entity.__fragment__(fragment_entity_link())
    _frag__as__Statement_citations_by_triple_id = _frag__as__Statement.citations_by_triple_id()
    _frag__as__Statement_citations_by_triple_id_nodes = _frag__as__Statement_citations_by_triple_id.nodes()
    _frag__as__Statement_citations_by_triple_id_nodes.url()
    _frag__as__Statement_qualifiers_by_subject_id = _frag__as__Statement.qualifiers_by_subject_id()
    _frag__as__Statement_qualifiers_by_subject_id_nodes = _frag__as__Statement_qualifiers_by_subject_id.nodes()
    _frag__as__Statement_qualifiers_by_subject_id_nodes.id()
    _frag__as__Statement_qualifiers_by_subject_id_nodes_predicate = _frag__as__Statement_qualifiers_by_subject_id_nodes.predicate()
    _frag__as__Statement_qualifiers_by_subject_id_nodes_predicate.name()
    _frag__as__Statement_qualifiers_by_subject_id_nodes.object_value()
    return _frag

class Fragment:
    triple_widget = fragment_triple_widget()

class Operations:
    fragment = Fragment

When running a query that uses this fragment, we get the error

{'errors': [{'message': 'Unknown fragment "EntityDetail".', ...
# and/or
{'message': 'Unknown fragment "TripleWidget".', ...

Digging into the generated operations, I noticed that the Operation objects don't seem to have the fragments included even though they should have been added through lines like _frag__as__Statement_subject.__fragment__(fragment_entity_link())

Does anyone know what seems to be going on here?

aychang95 commented 2 years ago

TL;DR for this issue:

The __fragment__() doesn't seem to store the fragment in the Operation object. Problem might have to do with the ... on Statement type condition.

@barbieri Do you have any ideas what might be going wrong here or if we're missing something?

barbieri commented 2 years ago

@aychang95 would need to dig into the code, nothing comes to my mind right now... but I'm out of time, so if you can debug and send a patch, I can review and apply (please include doctests).

A quick check in sgqlc/operations/__init__.py the code is handled as:

class SelectionList:
    def __fragment__(self, fragment):
        assert isinstance(fragment, Fragment)
        self.__fragments.setdefault(
            fragment.__type__.__name__, []
        ).append(fragment)
        self.__typename__()

So, if it's not crashing it's likely going in this function and what would be happening is the children __fragments is not being propagated to the parent... see class Fragment(SelectionList):, that is, the Fragment itself is ANOTHER selection list, maybe it's lacking some recursion. Seems __collect_fragments__ is doing it, but somewhere else may be lacking.

shiwen1209 commented 1 year ago

I'm experiencing the same issue. Used sgqlc codegen for custom operations, used fragments, and can see fragment in the generated file, but when running the query getting the same error "Unknown fragment". Is this resolved yet?

barbieri commented 1 year ago

I still don't have time to look into this, as I said above it looks like a propagation to the parent selection list (where the fragment is being used).

But I'd suggest to avoid this pattern of fragment inside fragment, at least until the bug is fixed. But it's not a common pattern to have immediate fragment-inside-a-fragment, I'm doing GraphQL client/server since 2017 and never had to use that, most of the time some co-worker used it was not the best approach and was blocked in code reviews.

That said, if you could take a look where the propagation is missing, let me know as that will save me lots of work