search

redwoodjs / redwood

The App Framework for Startups
https://redwoodjs.com
MIT License
17.14k stars 980 forks source link

API tests assume `api/src/lib/db` to export a Prisma client #3707

Open olance opened 2 years ago

olance commented 2 years ago

... and pretty much any Redwood command for that matter.

In my current project, I decided to go with Firebase's Firestore database.
So I'm building a Redwood app with an API side but where the exported db is just:

export const db = app.firestore()

I've got my services working well with this, so I can take advantage of all the GraphQL niceties from the web side.

However, on the API side, I'm facing a roadblock when writing/running tests for a custom function: image

It appears that, deep within Redwood, it is assumed that a Prisma client will always be used. Given the line that's failing (await db.$executeRawUnsafe(`DELETE from "${model}"`)) and the nature of supported databases from Prisma, I'd even say Redwood assumes a relational/SQL database is being used.

I'm aware that Prisma is the tool of choice for DB access/"ORM" in Redwood, but the fact that the DB connection object is actually part of the application code makes me want to be able to use something different all together (and since Prisma does not support Firestore, I actually don't have much of a choice).

So I'm wondering if something could be done to have the tests setup be agnostic of the nature of db?

Also, FWIW:

@ajcwebdev I saw your tutorial on Redwood + FaunaDB; did you get API side tests to work in any way?

ajcwebdev commented 2 years ago

I didn't get to the point of writing any tests for the Fauna project, it was mostly just a proof of concept to see if we could get it connected at all.

But I agree that we should definitely make it possible for anyone to swap out Prisma for their backend tool of choice. We've said for a while that this is possible so we need to make sure it actually is in practice.

dthyresson commented 2 years ago

I think the issue here is that testing the api side attempts to also

Both of which are Prisma-centric.

@cannikin Should there be an option to disable each? Maybe in api toml like scenarios = false?

cannikin commented 2 years ago

@dthyresson Yeah it seems like we need some way to disable. I think there was a sledgehammer solution at one point that looked to see if the db directory existed at all, and if not, don't try to generate Prisma and stuff, but if you have the dir, but don't use Prisma, you'll be having problems.

Although in @olance's case it sounds like he's trying to use scenarios, but not with Prisma, so we wouldn't want to disable them completely. Hmmm...

Maybe we can do an internal check for some property of db that's unique to Prisma, and only go forward wtih Prisma-related things if that property is present? Otherwise assume the person replaced Prisma with their own thing, and then they're on their own.

olance commented 2 years ago

@cannikin Yeah I'm OK with not using scenarios specifically: my blocker here is that I can't even run Jest because the setup code from @redwoodjs/testing tried to use db as a Prisma client.

The discussion above diverged a little towards other commands, which I think is part of the "package" if we want to make things perfectly agnostic of Prisma, but do have an easy workaround (i.e. keep the schema.prisma file with a dummy model).

Instead of looking at methods on the db object, I'd advocate for checking whether there's a Prisma schema file. I'd love to be able to delete it 🙃 But that will have profound implications on all generators too.

olance commented 2 years ago

I have started exploring the codebase for references to db where it is assumed it is a Prisma client... thought that might be a good starting point to evaluate both the amount of work and the correct path to a Prisma-agnostic "core", if that's something we'd like to achieve.

(no special order assumed in reporting those occurrences below)

handlePrismaLogging in packages/api/src/logger/index.ts

Very obvious function name, looks like simply a convenience provided by the framework to setup db logging easily. Looks to me like one could easily reproduce for their own DB if needed.

validateUniqueness in packages/api/src/validations/validations.ts

This service validation relies heavily on Prisma: it instantiates its own PrismaClient and then uses the $transaction method to check for uniqueness/execute callback.

runScript function in packages/cli/src/commands/exec.js

This is the main function for the exec command. It assumes a script being run with exec will have initialized a DB connection via Prisma and tries to close that connection with db.$disconnect().

packages/cli/src/commands/dataMigrate

This, obviously, is 100% Prisma specific as it uses Prisma Migrate to upgrade/downgrade the current DB schema.

packages/cli/src/commands/generate/service

The Service generator, both code and provided templates, assume the use of Prisma.

packages/cli/src/commands/setup/auth/templates

Templates for the dbAuth and ethereum (based on the former) both assume db to be a PrismaClient.

packages/create-redwood-app/template/api/src/functions/graphql.ts

The GraphQL handler tries to close the DB connection upon any thrown exception. It assumes db to be a PrismaClient and calls db.$disconnect()

packages/create-redwood-app/template/scripts/seed.ts

The template seed script assumes db to be a PrismaClient.

packages/testing/config/jest/api/jest.setup.js

packages/api/src/functions/dbAuth/DbAuthHandler.ts

The DbAuthHandlerOptions interface expects a db field to be a PrismaClient instance, and the authModelAccessor value to exist as a key from PrismaClient (i.e. to be a declared Prisma model).

packages/cli/src/commands

Following commands try to generate the Prisma Client as a preliminary step:

However they all rely on generatePrismaClient/generatePrismaCommand, which correctly skip when no Prisma schema file is present.

packages/cli/src/commands/deploy

Render, Netlify & Vercel deploy handlers expect DB & migrations to be handled with Prisma.

packages/cli/src/commands/setup/deploy/providers/aws-serverless.js

AWS Serverless deploy assumes the existence of the Prisma Schema file

packages/cli/src/commands/setup/deploy/deploy.js

Deploy setup command & providers assume the use of Prisma

packages/create-redwood-app/template

Templates for create-redwood-app all assume the use of Prisma.

packages/testing/src/api/scenario.ts

Scenarios assume Prisma to be used, and create generated models with a specific PrismaClient instance

I have probably missed some, and have of course omitted tests, tutorials, ...

So all in all, and with no surprises I guess, it turns out there's a lot of coupling to Prisma within the framework.

Reading Redwood's introduction & philosophy, I'd say it makes sense:

Redwood believes that there is power in standards, and makes decisions for you about which technologies to use

Redwood believes that traditional, relational databases like PostgreSQL and MySQL are still the workhorses of today's web applications and should be first-class citizens.

Prisma is definitely pushed as a "decision made for me" regarding the best database technology to use and makes Postgres & MySQL first-class citizens of the framework indeed.

However the next sentence is:

However, Redwood also shines with NoSQL databases like FaunaDB.

I don't think that holds true. As a user of a NoSQL database or, for that matter, any DB system not supported by Prisma, my experience with Redwood is quite degraded:

Note that this is not meant as a critique at all, but as a simple list of things that aren't easy or possible to do with Redwood when using NoSQL databases.

It is obvious that choices must be made and priorities must be set. If the current path forward is to go all in with Prisma, then maybe the only action to take could be to remove that sentence about NoSQL from Redwood Philosophy.

But if we'd like users of NoSQL/unsupported DB systems to be able to use Redwood, then there's some technical debt to be paid. From the little exploration I just did, it looks like we'd need an abstraction layer between Redwood's internals and the DB connection/features. It's mostly about closing the DB connection, listing declared models, deleting them, ...

To that end, we might want to borrow ideas from Ruby on Rails; they've managed to build strong primitives in the core framework, with amazing extensibility capabilities at all levels (generators, models, ORM, ...) But I know you're aware of that already 😉

jtoar commented 2 years ago

@olance as always your write ups are great—super detailed and easy to follow! As you've correctly deduced, we depend on prisma a lot. And before all the investigative work you did, I would've said "you probably can't just take it out and expect everything else to work". But now I can say that you definitely can't!

The decoupling we've focused on so far as a framework has been between the web side and the api side. While I think you can run the web side and the api side independently, I'm not sure that I'd say it's easy to do yet. It's definitely not documented. And to my knowledge we don't have any tests that explicitly ensure this is possible. But I do know plenty of users running a CLI or Next.js app alongside their Redwood app. I think we'll formally focus on the web/api side decoupling when we add another side, like CLI or mobile.

What you've brought up here is about a deeper level of decoupling—"intra" side decoupling; making things more composable within api, within web. While we aspire to this, we've definitely prioritized making things work over making things composable. The graphql-server was a step towards composability, but other than that there hasn't been much progress. You're right that the README touts the framework as if it did have these features today though. It was written first and was supposed to be aspirational, but given that it's been a year or two, it could use a bit of a rewrite.

Along the lines of strong primitives in the core framework, with amazing extensibility capabilities at all levels: have you seen this?

It's supposed to be a "stealth" feature (i.e. planned for v2, or just post v1-rc). It could help us give users the option to use something other than Prisma while still being plugged into Redwood.

The list you compiled will be invaluable when we begin to seriously focus on decoupling things. It's on our minds, but unfortunately not on our list of v1 priorities. But I can promise you that there'll be many versions after v1!

thedavidprice commented 2 years ago

+1 to @jtoar's comments above. And, again, thank you @olance for helping us make this better (even if simply diagnosing, in detail, what isn't working).

We have the v1-rc coming out SuperSoon. I've added a task to my "post v1-rc list" about doing a full audit of the current decoupling capability, using this Issue as a reference.

olance commented 2 years ago

Hey guys, thank you for the comments! And of course, I'm always happy to help, even if irregularly ^^

@jtoar thanks for the draft PR link, it looks very promising, very Rails-like (which I can only love) and exactly the kind of decoupling capabilities I had in mind. But you are right in your analysis, it's a very deep level of decoupling and I know it makes sense to get there later, and progressively.

But I can promise you that there'll be many versions after v1!

I know! 😄 Looking forward to v1 ^^

olance commented 2 years ago

Also, we can probably close this for now then?

I guess I'll add dummy methods to my exported db and that'll work around the tests not running!