Skip to content
This repository has been archived by the owner on Jan 14, 2021. It is now read-only.

Middlewares #770

Closed
timsuchanek opened this issue Jul 8, 2020 · 19 comments · Fixed by prisma/prisma#3029
Closed

Middlewares #770

timsuchanek opened this issue Jul 8, 2020 · 19 comments · Fixed by prisma/prisma#3029
Assignees
Labels
kind/feature A request for a new feature. tech/typescript Issue for tech TypeScript. topic: middleware topic: previewFeatures
Milestone

Comments

@timsuchanek
Copy link
Contributor

timsuchanek commented Jul 8, 2020

Related to the discussion in #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:

  • Monitoring, measuring performance, logging
  • Overwriting args
  • Validation

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.

@timsuchanek timsuchanek added kind/feature A request for a new feature. process/candidate Candidate for next Milestone. tech/typescript Issue for tech TypeScript. labels Jul 8, 2020
@thebiglabasky thebiglabasky added the roadmap/planned Planned for being picked up once we're done with the WIP label Jul 8, 2020
@janpio janpio added this to the 2.3.0 milestone Jul 8, 2020
@matthewmueller
Copy link
Contributor

matthewmueller commented Jul 8, 2020

Other usecases:

  • Caching certain queries
  • Mocking out parts of the client
  • Studio could migrate from hooks to this

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

@yoshuawuyts
Copy link

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.

@Jolg42 Jolg42 assigned Jolg42 and timsuchanek and unassigned Jolg42 Jul 8, 2020
@janpio janpio removed the process/candidate Candidate for next Milestone. label Jul 8, 2020
@thebiglabasky thebiglabasky removed the roadmap/planned Planned for being picked up once we're done with the WIP label Jul 9, 2020
@timsuchanek
Copy link
Contributor Author

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

@matthewmueller
Copy link
Contributor

matthewmueller commented Jul 13, 2020

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

  • Should next come second? Could next be optional? Would you ever just want the query? Would you ever just want next without the query?

@yoshuawuyts
Copy link

@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
Copy link
Contributor

matthewmueller commented Jul 13, 2020

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
Copy link

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
Copy link
Contributor Author

timsuchanek commented Jul 14, 2020

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

  • Makes it clearer when reading the code, that it is after all passed into the query function

Con

  • Not minimal

B

Pro

  • More minimal
  • Proven in Koa

Con

  • Potentially unclear

C

Pro

  • As with Koa, this allows manipulating the response. (which is crucial in Koas case but not in ours)

Con

  • Not yet sure what the problems of this approach could be

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

@matthewmueller
Copy link
Contributor

matthewmueller commented Jul 14, 2020

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

matthewmueller commented Jul 14, 2020

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

@Jolg42
Copy link
Contributor

Jolg42 commented Jul 15, 2020

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
Copy link

thebiglabasky commented Jul 15, 2020 via email

@matthewmueller
Copy link
Contributor

matthewmueller commented Jul 15, 2020

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
Copy link
Contributor Author

@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
Copy link

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 added a commit to prisma/prisma that referenced this issue Jul 15, 2020
timsuchanek added a commit to prisma/prisma that referenced this issue Jul 15, 2020
timsuchanek added a commit to prisma/prisma that referenced this issue Jul 16, 2020
timsuchanek added a commit to prisma/prisma that referenced this issue Jul 16, 2020
timsuchanek added a commit to prisma/prisma that referenced this issue Jul 16, 2020
timsuchanek added a commit to prisma/prisma that referenced this issue Jul 16, 2020
@timsuchanek
Copy link
Contributor Author

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
Copy link
Contributor Author

You can check out more examples here and here

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
kind/feature A request for a new feature. tech/typescript Issue for tech TypeScript. topic: middleware topic: previewFeatures
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants