prisma / prisma-client-js

Type-safe database client for TypeScript & Node.js (ORM replacement)
Apache License 2.0
1.47k stars 67 forks source link

Middlewares #770

Closed timsuchanek closed 4 years ago

timsuchanek commented 4 years ago

Related to the discussion in https://github.com/prisma/prisma-client-js/issues/669

Users of Prisma Client need a way to hook into Prisma Client. The system that allows this hooking-in needs to allow the following use-cases:

When having hooks in a system like Prisma Client, we can differentiate between blocking and non-blocking hooks. Blocking would mean - this function needs to succeed before we continue with executing the Prisma Client query. Non-blocking would be a traditional event emitter, for example used for logging. It will be called async and doesn't have to succeed for the client to continue with its business.

Why am I calling this a middleware instead of a hook? A middleware clearly states, that it is the "blocking thing" that can impact the control flow of what Prisma Client does.

As a first start, we should think about a simple but effective API, that solves as many use-cases as possible, while minimizing the API surface.

The API I suggest for that looks like this:

prisma.use(async ({ fetchQuery, params }) => {
  console.log('Before')
  const data = await fetchQuery(params)
  console.log('After')
})

This hook or middleware will then be called before every query, for example prisma.user.findMany(). The params include all the relevant information, like the args, the model and the method being used. Details of the params have to be figured out.

With this approach, we can cover all the above-mentioned use-cases.

Please comment, if this API would be useful for you and if any use-cases are left out by this.

matthewmueller commented 4 years ago

Other usecases:

What is fetchQuery in this case? Does this API work with creates, updates, and deletes?

yoshuawuyts commented 4 years ago

We have a similar approach for the Rust Surf HTTP client, which was inspired by koa's middleware approach -- overall this is working out quite well for us and excited for a similar approach here as well.

timsuchanek commented 4 years ago

@matthewmueller fetchQuery is the function that does the execution. Would execute make more sense?

matthewmueller commented 4 years ago

Yah I think so. I also think next would make it more clear that it's similar to Koa/Express. Do we also need the object syntax?

Express

app.all('/secret', function (req, res, next) {
  console.log('Accessing the secret section ...')
  next() // pass control to the next handler
})

Koa

app.use(async (ctx, next) => {
  const start = Date.now();
  await next();
  const ms = Date.now() - start;
  console.log(`${ctx.method} ${ctx.url} - ${ms}ms`);
});

Proposal

prisma.use(async (query, next) => {
  console.log('Before')
  const data = await next(query)
  console.log('After')
})

Open Questions with Proposal

yoshuawuyts commented 4 years ago

@matthewmueller in the proposal what should be done with data? Should it be returned from the closure so it can be handed to the next middleware?

matthewmueller commented 4 years ago

Great point! What are some use case where you'd want to manipulate the returned data in middleware?

One example is perhaps unnesting the data. Something like: https://github.com/paularmstrong/normalizr

I also wonder if it's possible to always have access to the latest data from middleware. Then we might not need anything (not sure if this is a good idea.)

prisma.use(async (query, next) => {
  const data = await next(query)
  console.log(data) // would data be the transformed or not?
})

prisma.use(async (query, next) => {
  const data = await next(query)
  transform(data) 
})
yoshuawuyts commented 4 years ago

Great point! What are some use case where you'd want to manipulate the returned data in middleware?

@matthewmueller good question. If we consider prisma-client to effectively be an HTTP client then things like redirects, validation, setting/storing cookies, extracting headers are all valid things to want to do.

Manipulating the actual returned data; I'm not sure. Perhaps there are some interesting analysis we could do (instrument size if tracing enabled). But in a pattern as general as middleware I would generally err on the side of flexibility.

timsuchanek commented 4 years ago

I like the idea of next and the fact that this can be a similar API to Koa.

The next function in Koa indeed returns a Promise of undefined. So you can't change the "return" of the HTTP server like that. However, that also doesn't make sense in Koa's case, due to the nature of HTTP, where you might instead send back a stream. Instead with Koa you use the ctx object to "return something"

@matthewmueller making your proposal even a bit more koa-like, we could also do this:

A: query is required

prisma.use(async (query, next) => {
  query.args.first = 10
  await next(query)
})

B: query can't even be passed into next

prisma.use(async (query, next) => {
  query.args.first = 10
  await next()
})

C: next returns the data, query is required

prisma.use(async (query, next) => {
  query.args.first = 10
  const data = await next(query)
  return data
})

What do you think about that? It looks like there are trade-offs for the solutions.

A

Pro

Con

B

Pro

Con

C

Pro

Con

I don't really have a favorite yet, they all have their tradeoffs.

matthewmueller commented 4 years ago

Thanks organizing this discussion @timsuchanek. I'm slightly in favor of C, but one thing I can't reconcile with is what happens to that return data?

For example,

// called first, then last, koa-style
prisma.use(async (query, next, data) => {
  console.log(data) // undefined
  await next(query)
  console.log(data) // defined
})

prisma.use(async (query, next) => {
  query.args.first = 10
  const data = await next(query)
  return data
})

It's a bit weird. Any ideas? Or maybe I'm missing what you mean @yoshuawuyts by return data? Do you mean it either gets returned to the next one or ignored?

Sidenote Blake has a nice generic implementation of this https://github.com/blakeembrey/compose-middleware. Edit doesn't look promise ready.

thebiglabasky commented 4 years ago

I'd go with C. One use case: the middleware uses the updated data (e.g. autogenerated IDs...) to store it in a data warehouse for analytical purpose.

timsuchanek commented 4 years ago

Sounds good! @matthewmueller the return data should stay mandatory, which can be enforced by TypeScript.

Let's think a bit how the API will work:

client.use(async (query, next) => {
  console.log('1')
  query.x = 2
  const data = await next(query)
  console.log('4')
  return data
})

client.use(async (query, next) => {
  console.log('2')
  query.x = 3
  const data = await next(query)
  console.log('3')
  return data
})

This would print

1
2
3
4

And the query object would have x: 3, as the second middleware is called after the first one, basically how the cascading is described here: https://koajs.com/

Here is a prototype implementation: https://gist.github.com/timsuchanek/971d39047348e27190f667b8811f9d52

matthewmueller commented 4 years ago

Ah excellent. That makes sense now. API looks good to me. Thanks!

Jolg42 commented 4 years ago

I'm quite confused by having next and return because the Express one doesn't have a return at all. It is used like this:

app.use((req, res, next) => {
  console.log('This is third middleware')
  next()
})

If we want to keep the return data then renaming next would be better I think.

thebiglabasky commented 4 years ago

True. Something like proceed or execute may sound better, prisma being pretty different than other libraries mentioned here as a reference for using next.

matthewmueller commented 4 years ago

I don't have a huge preference on this. It's an advanced API and I think people will figure it out, but I'm also okay to rename it.

Of the options provided, I don't think execute is the right word since it's more like passing query forward in the chain.

proceed, forward, etc. are synonyms of next. I think I'd prefer the simplest possible word, but I'm okay with any of these. One thing to keep in mind is that next is controlled by the user, they can rename it to whatever they want. We'll need to decide something for docs though.

timsuchanek commented 4 years ago

@Jolg42 I was also confused a bit and agree in Express terminology it doesn't make too much sense, but from the koa perspective, next is quite natural.

It seems that the main contestants are next and execute, but I agree with @matthewmueller, that execute doesn't denote the "user control" nature of this. execute would make sense, if there is only one middleware, as what you're then calling is indeed the execution of the Prisma Client query.

But as soon as you have multiple middlewares, you literally call the next one.

thebiglabasky commented 4 years ago

The only bit which puts me slightly against next is that it doesn't denote the fact that it actually performs the call on top of passing the ball to the next middleware (if any). That's why I was siding more with something telling that the actual call which is wrapped in the middleware will actually be executed.

I found this middleware approach looking like aspect-oriented programming, where the terminology is more around invoke or proceed. Now that may not resonate with the JS way as much, so I'll leave the group decide.

timsuchanek commented 4 years ago

Middlewares are implemented in 2.3.0-dev.38.

This is how to use them:

import { PrismaClient } from '@prisma/client'

async function main() {
  const client = new PrismaClient()

  client.use(async (params, next) => {
    console.log(params)
    return next(params)
  })

  const data = await client.user.findMany({})
  client.disconnect()
}

main()

This will log the following:

{
  args: {},
  dataPath: [],
  runInTransaction: false,
  action: 'findMany',
  model: 'User'
}
timsuchanek commented 4 years ago

You can check out more examples here and here