guacsec / guac

GUAC aggregates software security metadata into a high fidelity graph database.
https://guac.sh
Apache License 2.0
1.29k stars 176 forks source link

[feature] Enable pagination for GUAC GraphQL APIs #1525

Closed lumjjb closed 3 weeks ago

lumjjb commented 12 months ago

Is your feature request related to a problem? Please describe. Pagination as a feature has been brought up multiple times. For example, for data display and also to enable faster queries (i.e. prevent UI stalling).

Describe the solution you'd like Enable certain queries that may return large amounts of data to allow pagination.

Describe alternatives you've considered NA

Additional context Add any other context or screenshots about the feature request here.

pxp928 commented 11 months ago

The Relay-style pagination seems to be the most recommended and compatible with Apollo and can also be used by REST.

Note: ApolloClient can be used to enable caching of the results.

The graphQL schema needs to be updated with the following (per noun and verb):

type ArtifactConnection {
    totalCount: Int!
    pageInfo: PageInfo!
    edges: [ArtifactsEdge!]!
}

type ArtifactsEdge {
  cursor: ID!
  node: Artifact
}

extend type Query {
  "Returns all artifacts matching a filter."
  artifacts(artifactSpec: ArtifactSpec!): [Artifact!]!
 "Returns all artifacts matching a filter in chucks based on cursor"
  artifactsList(artifactSpec: ArtifactSpec!, after: ID, first: Int): ArtifactConnection
}

with PageInfo defied as below:

type PageInfo {
    hasNextPage: Boolean!
    startCursor: ID
    endCursor: ID
}

With the Edge, Connection, node and pageInfo defined as:

Example test implementation for the key/value backend can be found here: https://github.com/guacsec/guac/compare/main...pxp928:artifact-ff:test-pagination-artifact?expand=1

Example implementation with ENT: https://betterprogramming.pub/clean-architecture-with-ent-and-gqlgen-a789933a3665

Other useful docs: https://medium.com/@chris.czurylo/implementing-pagination-in-graphql-and-go-using-gqlgen-2ea3786a71dc https://www.apollographql.com/blog/graphql/pagination/understanding-pagination-rest-graphql-and-relay/

mihaimaruseac commented 11 months ago

This is what I was planning to do for pagination too. Initially I thought we might want to define a custom paginationSpec argument and just add it to all queries, but, while this is simple (one single change, one single place in code to update), it does not follow the standard so it won't benefit from caching, etc.

Let's do the right thing and follow the process @pxp928 mentioned above. I think in the long term, artifacts will be the one that has pagination, replacing artifactsList, but for now let's use both to not break users while we migrate

pxp928 commented 7 months ago

Based on further discussion, removing before: ID, last: Int and hasPreviousPage: Boolean as it will not be utilized and add unnecessary complexity to the codebase. The other values remain as they are. The plan is to add artifactsList in one PR and remove artifacts in the following PR by replacing the functionality of artifactsList with artifacts (making any updates as necessary).

pxp928 commented 6 months ago

The only queries that have not been implemented with pagination yet are: FindSoftware and Neighbors

pxp928 commented 3 weeks ago

Closing this as pagination is completed for the queries that need it.