movio / bramble

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

fix: :alien: Added support for nodes query #217

Open karatekaneen opened 1 year ago

karatekaneen commented 1 year ago

Ent (sometimes?) generates a "nodes" query to fetch multiple nodes in batch in the latest version . This was not compatible with Bramble so I fixed that.

The queries that I get generated in my project looks like this:

type Query {
  """Fetches an object given its ID."""
  node(
    """ID of the object."""
    id: ID!
  ): Node
  """Lookup nodes by a list of IDs."""
  nodes(
    """The list of node IDs."""
    ids: [ID!]!
  ): [Node]!
}

Maybe related to #96, not sure

pkqk commented 1 year ago

Hi @karatekaneen is this the ent you mean? Can you post an example of generating the graphql nodes field.

The support for the Relay Node interface is relatively unused at the moment, it's left over from an initial prototype design of how our federation model would work.

codecov-commenter commented 1 year ago

Codecov Report

Attention: 19 lines in your changes are missing coverage. Please review.

Comparison is base (07dc77d) 72.36% compared to head (db081a9) 72.43%. Report is 4 commits behind head on main.

Files Patch % Lines
validate.go 77.19% 11 Missing and 2 partials :warning:
config.go 0.00% 5 Missing :warning:
executable_schema.go 0.00% 1 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #217 +/- ## ========================================== + Coverage 72.36% 72.43% +0.07% ========================================== Files 27 27 Lines 2790 2848 +58 ========================================== + Hits 2019 2063 +44 - Misses 630 644 +14 Partials 141 141 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

karatekaneen commented 1 year ago

@pkqk Yes, it's Ent with entgql I'm referring to. The problem was that when I was trying to add my service to Bramble I got an error with something like "unknown type Node" which I wanted to get rid off.

My Ent schema is currently something of a kitchensink but it looks like this:

package schema

import (
    "entgo.io/contrib/entgql"
    "entgo.io/contrib/entproto"
    "entgo.io/ent"
    "entgo.io/ent/dialect/entsql"
    "entgo.io/ent/schema"
    "entgo.io/ent/schema/field"
)

type FormData struct {
    ent.Schema
}

func (FormData) Annotations() []schema.Annotation {
    return []schema.Annotation{
        entsql.Annotation{Table: "data"},
        entgql.RelayConnection(),
        entgql.Directives(entgql.Directive{Name: "boundary"}),
        entgql.QueryField(),
        entproto.Message(),
        entproto.Service(entproto.Methods(entproto.MethodGet)),
    }
}

// Fields of the FormData.
func (FormData) Fields() []ent.Field {
    return []ent.Field{
        field.Int("object_id").
            StorageKey("obj_id").
            Annotations(entproto.Field(2)),

        field.String("module").
            StorageKey("data_mod").
            Annotations(entproto.Field(3)),

        field.Int("type").
            StorageKey("data_type").
            Annotations(entproto.Field(4)),
    }
}

// Edges of the FormData.
func (FormData) Edges() []ent.Edge {
    return []ent.Edge{}
}

And the relevant parts of GraphQL that gets generated looks like this:

type FormData implements Node @boundary {
  id: ID!
  objectID: Int!
  module: String!
  type: Int!
}

interface Node @goModel(model: "bitbucket.org/company/entdemo/ent.Noder") {
  id: ID!
}

type Query {
  node(id: ID!): Node
  nodes(ids: [ID!]!): [Node]!
  formDataSlice(
    after: Cursor
    first: Int
    before: Cursor
    last: Int
    where: FormDataWhereInput
  ): FormDataConnection!
}

So, just to be clear. I don't mind the Relay Node interface to be unused. It's just that I wanted to be able to hook up my projects to Bramble without having to filter out the incompatible query from every schema in the service query.

karatekaneen commented 10 months ago

@pkqk Had any chance to take a look at this?

pkqk commented 10 months ago

Hi @karatekaneen I haven't had a chance to review it yet, though your use case makes more sense now. I might be able to find time end of next week. Are you ok running your fork in the mean time?

karatekaneen commented 9 months ago

@pkqk It's ok to run the fork for now. Just wanted to avoid syncing it when you are doing updates.

karatekaneen commented 8 months ago

@pkqk Found a small bug where the custom http.Client wasn't being respected. Fixed that on this branch as well

pkqk commented 8 months ago

Thanks @karatekaneen, would you be able to pull it out into it's own PR? I can merge and test that more quickly. We haven't had a chance to test with Ent yet.