graphql-nexus / nexus-plugin-prisma

Deprecated
MIT License
828 stars 118 forks source link

Field and query option to limit max page size in request #497

Open jhalborg opened 4 years ago

jhalborg commented 4 years ago

Hey! 👋

I think it'd be really great if Nexus Prisma could accept a query option for max page size (i.e. first param) for lists to prevent API consumers from performing too expensive queries on the system.

For example, given the following query

query UsersWithPosts {
  users(first:500) {
    posts { # each user might have a lot of posts
      id
      content
    }
  }
}

This query could be very, very expensive, especially if users have a lot of posts. There are of course other ways of handling this like query whitelisting, but I think it'd be great if Nexus Prisma could support putting in a sane default for max page size like 50 or 100 pr type/field, depending on the underlying model and domain. Ideally, the error message should also be easily customisable.

Edit See my two comments below for more context: https://github.com/prisma-labs/nexus-prisma/issues/497#issuecomment-544455313 https://github.com/prisma-labs/nexus-prisma/issues/497#issuecomment-544918563

Weakky commented 4 years ago

Hey 👋,

Nexus Prisma should already provide pagination supports by default on all list fields.

query UsersWithPosts {
  users(first:100) {
    posts(first: 100) { # each user might have a lot of posts
      id
      content
    }
  }
}

Above query should be valid, if it doesn't work for you, it's a bug.

What we could think about though is whether we want to make the first argument required or keep it optional. GitHub for instance makes the first arg always required on paginated lists.

jhalborg commented 4 years ago

The first argument works fine 😄 What I'm looking for is a way to set a default/max value possible to pass to first. Actually, it's both - it'd be great if

The point is to prevent clients from making non-paginated and very expensive requests, intended/malicious or inadvertently. Our current schema is generated using vanilla graphql-js, and here we've set limits and defaults for list arguments. We're planning a move to Nexus+Nexus-Prisma, and was looking for ways to do this for the generated lists, but couldn't find any so I created this issue 😄

It's not too critical as long as all clients/consumers are built in house alongside those who build the schema/backend, but it's a fairly large issue if you plan to allow external developers to start using the schema.

jasonkuhrt commented 4 years ago

Thanks for the feature request @jhalborg. Can you confirm I understood the feature request correctly: You would like a way to configure nexus-prisma to enforce runtime checks on the size pages requested in pagination?

Good idea!

jhalborg commented 4 years ago

Almost :-)

The default values could actually be passed to the generated schema, but the max values would have to be enforced during runtime.

Also, I think it's important to be able to provide a config for each, as well as field specific overrides.

Let's do a concrete example. If we have a schema like

type User {
  posts: [Post!]!
}

type Post {
  author: User!
  comments: [Comment!]!
}

type Comment {
  author: User!
  post: Post!
}

I would be like to tell Prisma Nexus that

Does that make sense? Not entirely sure if I get my point across 😄

jasonkuhrt commented 4 years ago

@jhalborg Crystal clear thanks!

ManAnRuck commented 4 years ago

this feature would be helpful :)

theneshofficial commented 4 years ago

Waiting for this :)

thalesagapito commented 3 years ago

If you're satisfied with limiting the take argument from the findMany actions, you can use the following middleware:

prisma.$use((params, next) => {
  const isFindMany = params.action === 'findMany';
  if (!isFindMany) return next(params);

  const takeParameter = params.args.take;
  const takeParameterIsUndefined = takeParameter === undefined;

  if (takeParameterIsUndefined || takeParameter > 30) {
    params.args.take = 30;
  }

  return next(params);
});

For my use case this is enough.