Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Skip empty types in union #2264

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Skip empty types in union #2264

wants to merge 2 commits into from

Conversation

jmini
Copy link
Contributor

@jmini jmini commented Jan 29, 2025

In #2224 I have explained why currently you need to have as many @JsonbSubtype entries in @JsonbTypeInfo as interface.

This allow to create a query like this:

query GetBooks {
  books {
    # __typename is added by the type-safe client to help with the deserialisation
    __typename
    ... on Textbook {
      title
      courses {
        # Only present in Textbook
        name
      }
    }
    ... on ColoringBook {
      title
      colors # Only present in ColoringBook
    }
  }
}

Source of the example: https://www.apollographql.com/docs/apollo-server/schema/unions-interfaces#querying-an-interface


In the Java client Book would be an interface and Textbook and ColoringBook the implementation.

Now if you are not interested at all by the attributes of ColoringBook you would want to create an empty class.

But currently the generated query is invalid, because it looks like this:

    ... on ColoringBook {
    }

This PR is fixing this by not producing any fragment if the Type does not have fields.

For my concrete case #2224, the goal would be to reduce the generated query:

Currently generated query

query workItemsByReference(
  $arg0: ID = "jmini/a_project"
  $arg1: [String!]! = ["#1"]
  $arg2: NotesFilterType = ONLY_COMMENTS
) {
  workItemsByReference(contextNamespacePath: $arg0, refs: $arg1) {
    count
    nodes {
      archived
      closedAt
      confidential
      createdAt
      id
      iid
      lockVersion
      namespace {
        fullPath
        id
        visibility
      }
      reference
      state
      title
      updatedAt
      webUrl
      widgets {
        __typename
        ... on WorkItemWidgetAssignees {
          assignees {
            nodes {
              id
              username
            }
          }
          type
        }
        ... on WorkItemWidgetAwardEmoji {
          type
        }
        ... on WorkItemWidgetColor {
          type
        }
        ... on WorkItemWidgetCrmContacts {
          type
        }
        ... on WorkItemWidgetCurrentUserTodos {
          type
        }
        ... on WorkItemWidgetDescription {
          description
        }
        ... on WorkItemWidgetDesigns {
          type
        }
        ... on WorkItemWidgetDevelopment {
          type
        }
        ... on WorkItemWidgetEmailParticipants {
          type
        }
        ... on WorkItemWidgetHealthStatus {
          type
        }
        ... on WorkItemWidgetHierarchy {
          ancestors {
            count
            nodes {
              archived
              confidential
              createdAt
              id
              iid
              namespace {
                fullPath
                id
                visibility
              }
              reference
              state
              title
              webUrl
              workItemType {
                id
                name
              }
            }
          }
          children {
            count
            nodes {
              archived
              confidential
              createdAt
              id
              iid
              namespace {
                fullPath
                id
                visibility
              }
              reference
              state
              title
              webUrl
              workItemType {
                id
                name
              }
            }
          }
          parent {
            archived
            confidential
            createdAt
            id
            iid
            namespace {
              fullPath
              id
              visibility
            }
            reference
            state
            title
            webUrl
            workItemType {
              id
              name
            }
          }
        }
        ... on WorkItemWidgetIteration {
          type
        }
        ... on WorkItemWidgetLabels {
          labels {
            count
            nodes {
              color
              description
              id
              textColor
              title
            }
            pageInfo {
              endCursor
              hasNextPage
              startCursor
            }
          }
          type
        }
        ... on WorkItemWidgetLinkedItems {
          linkedItems {
            nodes {
              linkCreatedAt
              linkId
              linkType
              linkUpdatedAt
              workItem {
                archived
                confidential
                createdAt
                id
                iid
                namespace {
                  fullPath
                  id
                  visibility
                }
                reference
                state
                title
                webUrl
                workItemType {
                  id
                  name
                }
              }
            }
          }
          type
        }
        ... on WorkItemWidgetMilestone {
          type
        }
        ... on WorkItemWidgetNotes {
          discussions(filter: $arg2) {
            nodes {
              id
              notes {
                nodes {
                  author {
                    id
                    username
                  }
                  awardEmoji {
                    nodes {
                      name
                      unicode
                      unicodeVersion
                    }
                  }
                  body
                  id
                }
              }
            }
          }
          type
        }
        ... on WorkItemWidgetNotifications {
          type
        }
        ... on WorkItemWidgetParticipants {
          type
        }
        ... on WorkItemWidgetRolledupDates {
          type
        }
        ... on WorkItemWidgetStartAndDueDate {
          dueDate
          startDate
          type
        }
        ... on WorkItemWidgetStatus {
          status
        }
        ... on WorkItemWidgetTimeTracking {
          type
        }
        ... on WorkItemWidgetWeight {
          type
        }
      }
      workItemType {
        id
        name
      }
    }
  }
}

This query is too big for the GraphQL server, but removing the non relevant widgets helps with the performance at server side:

Optimized query after this PR

query workItemsByReference(
  $arg0: ID = "jmini/a_project"
  $arg1: [String!]! = ["#1"]
  $arg2: NotesFilterType = ONLY_COMMENTS
) {
  workItemsByReference(contextNamespacePath: $arg0, refs: $arg1) {
    count
    nodes {
      archived
      closedAt
      confidential
      createdAt
      id
      iid
      lockVersion
      namespace {
        fullPath
        id
        visibility
      }
      reference
      state
      title
      updatedAt
      webUrl
      widgets {
        __typename
        ... on WorkItemWidgetAssignees {
          assignees {
            nodes {
              id
              username
            }
          }
          type
        }
        ... on WorkItemWidgetHierarchy {
          ancestors {
            count
            nodes {
              archived
              confidential
              createdAt
              id
              iid
              namespace {
                fullPath
                id
                visibility
              }
              reference
              state
              title
              webUrl
              workItemType {
                id
                name
              }
            }
          }
          children {
            count
            nodes {
              archived
              confidential
              createdAt
              id
              iid
              namespace {
                fullPath
                id
                visibility
              }
              reference
              state
              title
              webUrl
              workItemType {
                id
                name
              }
            }
          }
          parent {
            archived
            confidential
            createdAt
            id
            iid
            namespace {
              fullPath
              id
              visibility
            }
            reference
            state
            title
            webUrl
            workItemType {
              id
              name
            }
          }
        }
        ... on WorkItemWidgetLabels {
          labels {
            count
            nodes {
              color
              description
              id
              textColor
              title
            }
            pageInfo {
              endCursor
              hasNextPage
              startCursor
            }
          }
          type
        }
        ... on WorkItemWidgetLinkedItems {
          linkedItems {
            nodes {
              linkCreatedAt
              linkId
              linkType
              linkUpdatedAt
              workItem {
                archived
                confidential
                createdAt
                id
                iid
                namespace {
                  fullPath
                  id
                  visibility
                }
                reference
                state
                title
                webUrl
                workItemType {
                  id
                  name
                }
              }
            }
          }
          type
        }
        ... on WorkItemWidgetNotes {
          discussions(filter: $arg2) {
            nodes {
              id
              notes {
                nodes {
                  author {
                    id
                    username
                  }
                  awardEmoji {
                    nodes {
                      name
                      unicode
                      unicodeVersion
                    }
                  }
                  body
                  id
                }
              }
            }
          }
        }
      }
      workItemType {
        id
        name
      }
    }
  }
}

Query can be tested without authentication in https://gitlab.com/-/graphql-explorer

jmini added a commit to unblu/gitlab-workitem-graphql-client that referenced this pull request Jan 29, 2025
This requires the change discussed in PR smallrye/smallrye-graphql#2264
@jmini
Copy link
Contributor Author

jmini commented Jan 29, 2025

Test script for the query: WorkItemScript

Can be executed with:

jbang WorkItemScript.java --namespace "jmini/a_project" --ref "#1"

This query used to work with older gitlab versions, but now I get a Timeout on validation of query from the backend, so I need to simplify the generated query.

@jmini
Copy link
Contributor Author

jmini commented Jan 29, 2025

@jmartisk can you help me understand the CI?

It is now failing lines 10667:

Error:  Failures: 
Error:    UnionBehavior.shouldSkipEmptyTypes:156 
expected: "query details($name: String) { details(name: $name){__typename ... on SuperHeroDetailsData {data} } }"
 but was: "query details($name: String) { details(name: $name){__typename ... on SuperHeroDetailsData {data} ... on SuperHeroDetailsHidden {}} }"
[INFO] 
Error:  Tests run: 288, Failures: 1, Errors: 0, Skipped: 0

But on line 10488 I see that my test is passing:

[INFO] Running tck.graphql.typesafe.UnionBehavior
[INFO] Tests run: 5, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 0.027 s -- in tck.graphql.typesafe.UnionBehavior

So I assume the same test is executed in multiple context, but I don't get it.
I have the exact same behavior locally and it is very confusing for me.

@jmartisk
Copy link
Member

jmartisk commented Jan 29, 2025

I think you'll need to do the same change in https://github.com/smallrye/smallrye-graphql/blob/2.12.1/client/model-builder/src/main/java/io/smallrye/graphql/client/model/helper/OperationModel.java#L444 to accordingly update the build-time scanning.

We have two variants how the test is run - with the original implementation where queries are computed at runtime, and the newer 'build-time' variant where they are computed from the Jandex index during build. The classes in client/model-builder serve for the latter approach. The test runs in both variants, so that's why it passes in one and fails in the other. We need to try to keep both approaches equivalent.

@jmartisk
Copy link
Member

This build-time approach (the one you haven't updated yet) is, by the way, the default behavior in Quarkus, if you're using a declaratively defined GraphQL client (using configuration properties rather than a programmatic builder).

@jmini
Copy link
Contributor Author

jmini commented Jan 29, 2025

🎉 Makes totally sense. Thank you very much for your hint on this. I was really stuck.

jmini added a commit to unblu/gitlab-workitem-graphql-client that referenced this pull request Jan 30, 2025
This requires the change discussed in PR smallrye/smallrye-graphql#2264
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants