profusion / sgqlc

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

Shopify GraphQl non standard conform pageInfo. Way around it? #112

Closed ascheucher closed 3 years ago

ascheucher commented 3 years ago

Hi Gustavo!

Thanks for creating and maintaining this library! I am working on my first implementation using it to face the Shopify GraphQl API. Sadly, they don't care about details as much as they should.

Their pageInfo responses lack the startCursor and endCursor fields, which are mandatory following the GraphQl specifications.

Your library is implementing this perfectly correct, but fails on merging responses of paginated calls of connection edges, as it accesses the non existing field in the page_info object.

I tried to create a subclass adding the two fields, but obviously I do not understand how the schema.type objects are populated by sgqlc.

This is my implementation so far, just to give you an idea what I want to achieve:

# out of the schema file
class PageInfo(sgqlc.types.Type):
    __schema__ = schema
    __field_names__ = ('has_next_page', 'has_previous_page')
    has_next_page = sgqlc.types.Field(sgqlc.types.non_null(Boolean), graphql_name='hasNextPage')
    has_previous_page = sgqlc.types.Field(sgqlc.types.non_null(Boolean), graphql_name='hasPreviousPage')

# my subclass
class PageInfoWithStartAndEndCursor(schema.PageInfo):
    __schema__ = schema
    __field_names__ = ('start_cursor', 'end_cursor')
    start_cursor = sgqlc.types.Field(
        sgqlc.types.non_null(String), graphql_name='startCursor')
    end_cursor = sgqlc.types.Field(
        sgqlc.types.non_null(String), graphql_name='endCursor')

# my naive attempt to patch the sgqlc response
def patch_page_info(self, page_info_parent_connection: sgqlc.types.relay.Connection) -> sgqlc.types.relay.Connection:
    """
    Shopify does not follow the GraphQl specification: 
        https://relay.dev/graphql/connections.htm#sel-EALFDDAAACJvBi4D

    They omit he mandatory field pageInfo.startCursor and also 
    pageInfo.endCursor. Hence, the sgqlc JSON to object translation
    fails on paged results.

    This method creates a subclass of the given connection class with 
    the additional mandatory cursor fields to be standard compliant 
    again.
    """
    # create the patched connection
    patched_page_info = PageInfoWithStartAndEndCursor(
        startCursor=page_info_parent_connection.edges[0].cursor,
        endCursors=page_info_parent_connection.edges[-1].cursor)
    patched_page_info.has_next_page = page_info_parent_connection.page_info.has_next_page
    patched_page_info.has_previous_page = page_info_parent_connection.page_info.has_previous_page

    # use it as replacement of the original one
    page_info_parent_connection.page_info = patched_page_info
    return page_info_parent_connection

I tried to debug into your code how the objects are inflated, but it seems to be way over my head. Can you give me a hint, how I can mess around with your library to still be able to += the responses together?

To access the Shopify schema, it's this endpoint: https://${MY_SHOP}.myshopify.com/admin/api/2020-10/graphql.json As you need an Shopify shop to access them, here is a copy of the schema generated with your introspection tool.

Thanks for your effort!

Best, Andreas

barbieri commented 3 years ago

Hi @ascheucher,

My PageInfo and Connection stuff are optional helpers listed in https://github.com/profusion/sgqlc/blob/master/sgqlc/types/relay.py you can define something else. Maybe my tool is being "eager" to declare that dependency based on name, not the full interface? One option would be to tweak the generated code manually and see if manually declaring their PageInfo fixes it -- however the Connection helpers (like the __iadd__, aka +=) will be lost -- but you can do one specific for your project, it's basically getting the page and accessing the next.

However, adding the non-existent fields as you were trying won't help, as the information (cursors) is not available.

But looking at your schema (thanks for making it easy), it seems that FulfillmentEventEdge provides cursor, so what we could do is fallback to the first/last cursors when pageInfo lacks them. I'll try to fix that for you this week.

barbieri commented 3 years ago

@ascheucher I did create an account at Shopify, however I'm out of time to think of an example that would exercise this code path... could you send me something as baseline, then I'll host that alongside the github.

barbieri commented 3 years ago

let me know if that works for you. It's cumbersome to have to fetch all cursors, find the last edge and get it's cursor... but doable.