movio / bramble

A federated GraphQL API gateway
https://movio.github.io/bramble/
MIT License
497 stars 55 forks source link

Fragment not mapping fields for interface types #158

Closed dilip9292 closed 2 years ago

dilip9292 commented 2 years ago

Problem Statement

When an interface type is implemented in graphql schema, we are able to query with spread operator and on Type pattern. When we do it by using the Fragments it is not mapping fields inside it when multiple type of interface implemenations are being returned

In the following PR https://github.com/movio/bramble/pull/153/files in the test case If we fetch either Gadjet Or Gizmo it works. If we want to fetch a list of object where object could be either Gadject or Gizmo it would not work

Example Request

interface Book {
  title: String!
}

type Textbook implements Book {
  title: String! # Must be present
  author: String!
}

type Magazine implements Book {
  title: String! # Must be present
  publisher: String!
}

type Query {
  books: [Book!] # Can include Textbook objects
}

fragment TextBookFragment on Textbook {
      author 
}

fragment MagazineFragment on Magazine {
      publisher 
}
query GetBooks {
  books {
    __typename
    title
    ... TextBookFragment
    ... MagazineFragment
  }
}

Response

 {
    books : [
       {
             ___typename : "TextBook"
       }
      {
             ___typename : "Magazine"
       }
   ]
}

Problem:

The other fields are not mapped

lucianjon commented 2 years ago

Hi @dilip9292. could we get some more info like what version of bramble you're running and how your example is being run?

There is no reason that shouldn't work, I've created an equivalent test case that behaves as I'd expect and I believe that's even a pattern we use in production.

dilip9292 commented 2 years ago

@lucianjon Thank you for taking a look

We are using bramble 1.4.0 . I cannot give exact details but i can give an example. Graphql Resonse : Where Textbook and Magazine implements Book Interface

[
    {___typename : "TextBook", author: "abc",  title: "abc"},
    {___typename : "Magazine", publisher: "def", title: "abc"},
]

But bramble response after connecting graphql service is like

[
 {___typename : "TextBook"},
 {___typename : "Magazine"},
]
nmaquet commented 2 years ago

@dilip9292 We dug into this and suspect that the issue is that you have two different spellings for the Textbook type. One is Textbook and the other one is TextBook. Fixing the spelling seems to resolve the issue. Can you double check on your end and report back? Thanks!

dilip9292 commented 2 years ago

Hi @nmaquet This was just an example. However i tried writing a test case and one thing i could notice was if service does not return _bramble__typename then the issue is seen.

Currently my hypothesis is that Currently when we run a graphql service and put a gateway infront of if the, service would not know that there is a bramble gateway infront of it and it does not return _bramble__typename.

The test case below is eaxctly our case and it passes when service returns _bramble__typename

serviceC := testService{ schema: directive @boundary on OBJECT interface Snapshot { id: ID! name: String! }

    interface Gozmo {
        id: ID!
    }

    type GozmoImplementation implements Snapshot {
        id: ID!
        name: String!
        gozmos: [Gozmo!]!
    }

    type Gizmo implements Gozmo {
        id: ID!
        name: String!
    }
    type Gadget implements Gozmo {
        id: ID!
        name: String!
    }
    type Query {
        snapshots: [Snapshot!]!
    }`,
    handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        w.Write([]byte(`
            {
                "data": {
                    "snapshots": [
                        {
                            "id": "100",
                            "name": "foo",
                            "gozmos": [
                                { "__typename": "Gadget", "id": "GADGET1" },
                                { "__typename": "Gizmo", "id": "GIZMO1" }
                            ],
                            "_bramble__typename": "GozmoImplementation"
                        }
                    ]
                }
            }`))
    }),
}`

`t.Run("test with multiple implementation fragment spreads (gadget implementation)", func(t *testing.T) {
    f := &queryExecutionFixture{
        services: []testService{serviceC},
        query: `
        query Foo {
            snapshots {
                ... NamedFragment
            }
        }

        fragment GizmoFragment on Gizmo {
            id
        }

        fragment GadgetFragment on Gadget {
            id
        }

        fragment GozmoImplementationsFragment on Gozmo {
            __typename
            ... GizmoFragment
            ... GadgetFragment
        }

        fragment NamedFragment on GozmoImplementation {
            id
            name
            gozmos {
                ...GozmoImplementationsFragment
            }
        }`,
        expected: `
        {
            "snapshots": [
                {
                    "id": "100",
                    "name": "foo",
                    "gozmos": [
                        {
                            "__typename": "Gadget",
                            "id": "GADGET1"

                        },
                        {
                            "__typename": "Gizmo",
                            "id": "GIZMO1"
                        }
                    ]
                }
            ]
        }`,
    }

    f.checkSuccess(t)
})`
lucianjon commented 2 years ago

@dilip9292 __bramble_typename is injected into the selection set passed to the downstream service during the planning stage of a bramble request (https://github.com/movio/bramble/blob/cc52747c89166c13ae2ae3fc61032c32ed481020/plan.go#L176). Since we're just using standard graphql, any spec compliant graphql service will return the result bramble expects.

dilip9292 commented 2 years ago

@lucianjon Thank you for validating my hypothesis. I have done a small check i took the above testcase and ran it aginst the latest code base and it works. I have reverted changes done on May 4 and ran again it failed. Since the release tag(1.4.0) was created on May 2nd we might need an another release. What do you think?

Test case of concern

serviceC := testService{ schema: directive @boundary on OBJECT interface Snapshot { id: ID! name: String! }

interface Gozmo {
    id: ID!
}

type GozmoImplementation implements Snapshot {
    id: ID!
    name: String!
    gozmos: [Gozmo!]!
}

type Gizmo implements Gozmo {
    id: ID!
    name: String!
}
type Gadget implements Gozmo {
    id: ID!
    name: String!
}
type Query {
    snapshots: [Snapshot!]!
}`,
    handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
        w.Write([]byte(`
        {
            "data": {
                "snapshots": [
                    {
                        "id": "100",
                        "name": "foo",
                        "gozmos": [
                            { "__typename": "Gadget", "id": "GADGET1" },
                            { "__typename": "Gizmo", "id": "GIZMO1" }
                        ],
                        "_bramble__typename": "GozmoImplementation"
                    }
                ]
            }
        }`))
    }),
}

t.Run("test with multiple implementation fragment spreads (gadget implementation)", func(t *testing.T) {
    f := &queryExecutionFixture{
        services: []testService{serviceC},
        query: `
    query Foo {
        snapshots {
            ... NamedFragment
        }
    }

    fragment GizmoFragment on Gizmo {
        id
    }

    fragment GadgetFragment on Gadget {
        id
    }

    fragment GozmoImplementationsFragment on Gozmo {
        __typename
        ... GizmoFragment
        ... GadgetFragment
    }

    fragment NamedFragment on GozmoImplementation {
        id
        name
        gozmos {
            ...GozmoImplementationsFragment
        }
    }`,
        expected: `
    {
        "snapshots": [
            {
                "id": "100",
                "name": "foo",
                "gozmos": [
                    {
                        "__typename": "Gadget",
                        "id": "GADGET1"

                    },
                    {
                        "__typename": "Gizmo",
                        "id": "GIZMO1"
                    }
                ]
            }
        ]
    }`,
    }

    f.checkSuccess(t)
})`
lucianjon commented 2 years ago

@dilip9292 That would explain it, I thought we had that fix in our latest release. Have released https://github.com/movio/bramble/releases/tag/v1.4.1. Will close this for now but feel free to reopen if you have issues on v1.4.1.