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

QueryDslDataFetcher and QueryByExampleDataFetcher return empty property selection list for cursor connection #723

Closed
meistermeier opened this issue Jun 14, 2023 · 6 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@meistermeier
Copy link
Contributor

meistermeier commented Jun 14, 2023

While I am doing the last steps to also bring complete scrolling support for Spring GraphQL into Spring Data Neo4j, I came across this line:

queryToUse = queryToUse.project(buildPropertyPaths(env.getSelectionSet(), resultType));

Given a (partial) schema like this:

type Query {
    movies(first:Int, after:String, last:Int, before:String): MovieConnection
}
type MovieConnection {
    edges: [MovieEdge]!
    pageInfo: PageInfo!
}
type MovieEdge {
    node: Movie!
    cursor: String!
}
type Movie {
    id: ID!
    #...
    actors: [Person]
    directors: [Person]
}

and an actual query like this:

{
  movies(first:3){
    pageInfo {
      hasPreviousPage
      hasNextPage
      endCursor
    }
   edges {
      cursor
      node {
        title
        id
        actors
        {
          name
        }
      }
    }
  }
}

The selection set of course contains the requested paths that does not match up with the entity (Movie) at all. In fact the paths look like MovieConnection.edges/MovieEdge.node/Movie.actors . As a consequence buildPropertyPaths will return 0 properties.
In Spring Data Neo4j we process those properties to reduce the amount of data and related nodes/entities that gets loaded. But in this case we can only return "everything" because there is no filter in place.
Would it be possible to make the method buildPropertyPaths resp. the underlying methods aware of the cursor connection? Or issue another subsequent call before execution with this meta-information (I am aware that this would require a new method in Spring Data Commons)?

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 14, 2023
@rstoyanchev
Copy link
Contributor

rstoyanchev commented Jun 14, 2023

Thanks for bringing this up. Small typo perhaps, in the sample, the query should be movies and not moviesPagination I think. I haven't investigated, but can see how this would be a problem, and will have to see why existing tests like this one don't reveal the issue.

@rstoyanchev rstoyanchev self-assigned this Jun 14, 2023
@rstoyanchev rstoyanchev added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 14, 2023
@rstoyanchev rstoyanchev added this to the 1.2.1 milestone Jun 14, 2023
@meistermeier
Copy link
Contributor Author

(yes, it was a typo. copied from my larger example)
The test does not uncover this behaviour because the Author is embedded in the Book document.
And it is hard to see from a Spring GraphQL perspective (to be more precise tests ;) ) because it happens on the query level and Spring GraphQL just forms the result matching and dismisses the superfluous fields / nested objects from the result.
Just pasting the query from the logs of the test into a mongo shell returns this.

docker-rs [direct: primary] test> db.book.find({"_class": {"$in": ["org.springframework.graphql.data.query.mongo.Book"]}})
[
  {
    _id: '1',
    name: 'Nineteen Eighty-Four',
    author: { _id: '0', firstName: 'George', lastName: 'Orwell' },
    _class: 'org.springframework.graphql.data.query.mongo.Book'
  },
  {
    _id: '2',
    name: 'The Great Gatsby',
    author: { _id: '0', firstName: 'F. Scott', lastName: 'Fitzgerald' },
    _class: 'org.springframework.graphql.data.query.mongo.Book'
  },
  {
    _id: '3',
    name: 'Catch-22',
    author: { _id: '0', firstName: 'Joseph', lastName: 'Heller' },
    _class: 'org.springframework.graphql.data.query.mongo.Book'
  },
  {
    _id: '42',
    name: "Hitchhiker's Guide to the Galaxy",
    author: { _id: '0', firstName: 'Douglas', lastName: 'Adams' },
    _class: 'org.springframework.graphql.data.query.mongo.Book'
  },
  {
    _id: '53',
    name: 'Breaking Bad',
    author: { _id: '0', firstName: '', lastName: 'Heisenberg' },
    _class: 'org.springframework.graphql.data.query.mongo.Book'
  }
]

@rstoyanchev
Copy link
Contributor

That makes sense. I'll have a closer look soon, with 1.2.1 coming up next week.

@meistermeier
Copy link
Contributor Author

No hurry, I don't even know if the other Spring Data modules respect filtering properties for entities/non-projections. I mean it's not a bug per se but could at least help us to optimise the query and only fetch the data that is requested (as it already happens for all other parts of SDN).

rstoyanchev added a commit that referenced this issue Jun 20, 2023
@rstoyanchev rstoyanchev changed the title Empty property list for cursor connection in QueryByExampleDataFetcher QueryDslDataFetcher and QueryByExampleDataFetcher return empty property selection list for cursor connection Jun 20, 2023
@rstoyanchev
Copy link
Contributor

There is a fix for this now. I've also added a test, so it should work as expected.

@meistermeier
Copy link
Contributor Author

Thank you very much. Cannot wait to try this out (with the release version).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

3 participants