Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug?]: Multi-Schema Support in Jest Setup for Multi-Schema Prisma project #10009

Open
1 task done
zaiyou12 opened this issue Feb 14, 2024 · 4 comments
Open
1 task done

Comments

@zaiyou12
Copy link
Contributor

zaiyou12 commented Feb 14, 2024

What's not working?

Description

In multi-schema Prisma projects, the teardown process in Jest setup files does not dynamically handle schema prefixes, leading to issues when trying to clean up test data. This is relevant for projects that using Prisma's preview feature multiSchema.

(In fact, there's nothing wrong with using only single schema. I just got greedy to use Prisma's preview feature, as the number of DB tables grew 🤣)

// api/db/schema.prisma
datasource db {
  provider = "postgres"
  url      = env("DATABASE_URL")
  schemas  = ["public", "cms"] // <-- Add "cms" schema
}

generator client {
  provider        = "prisma-client-js"
  binaryTargets   = "native"
  previewFeatures = ["multiSchema"] // <-- Add "multiSchema" feature
}

model User {
  id    Int     @id @default(autoincrement())
  email String  @unique
  name  String?

  @@schema("public") // <-- Use default schema
}

model Post {
  id    Int    @id @default(autoincrement())
  title String
  body  String

  @@schema("cms") // <-- Use new schema
}

Problem

The original teardown logic in the Jest setup file does not account for models located in different schemas, resulting in errors during the test cleanup phase. Specifically, the error occurs when attempting to delete data from tables without specifying the correct schema prefix, leading to failed tests due to unresolved table references.

// @redwoodjs/testing/config/jest/api/jest.setup.js
const configureTeardown = async () => {
  const { getDMMF } = require('@prisma/internals')
  const fs = require('fs')

  const schema = await getDMMF({ datamodelPath: dbSchemaPath })
  const schemaModels = schema.datamodel.models.map((m) => m.dbName || m.name) // <-- Only get table name without schema
  // ...
}
// ...
const teardown = async () => {
  const fs = require('fs')

  const quoteStyle = await getQuoteStyle()

  for (const modelName of teardownOrder) {
    try {
      await getProjectDb().$executeRawUnsafe(
        `DELETE FROM ${quoteStyle}${modelName}${quoteStyle}` // <-- Cannot find Post table in cms schema and throw error
      )
    } catch (e) {
      // ...
  }
}

Error

PrismaClientKnownRequestError:
Invalid `prisma.$executeRawUnsafe()` invocation:

Raw query failed. Code: `42P01`. Message: `relation "Post" does not exist`

Quick fix

To address this issue, I've made quick fix to the jest.setup.js configuration to dynamically extract schema information from the Prisma schema file and apply this information during the teardown process. This ensures that raw SQL commands are executed against the correct schema, allowing for proper cleanup of test data across multiple schemas.

Changes Made

  • Extract Model Schemas: Since getDMMF doesn't return schema with table name, I need to implement a function extractModelSchemas to parse the Prisma schema file and extract model-to-schema mappings. This function uses regular expressions to find model definitions and their @@Schema annotations.
  • Update Teardown Logic: Modified the teardown function to utilize the extracted schema information, prefixing SQL commands with the correct schema name.
// @redwoodjs/testing/config/jest/api/jest.setup.js
// ...
// Add new function - Function to read the schema file and extract model schema names
const extractModelSchemas = (schemaFilePath) => {
  const fs = require('fs')
  const schemaContent = fs.readFileSync(schemaFilePath, { encoding: 'utf8' });
  const modelSchemaMap = {};

  // Regular expression to match model blocks and their names
  const modelBlockRegex = /model\s+(\w+)\s+\{[^}]+\}/g;
  // Regular expression to extract the @@schema directive
  const schemaDirectiveRegex = /@@schema\("([^"]+)"\)/;

  let match;
  while ((match = modelBlockRegex.exec(schemaContent)) !== null) {
    const modelName = match[1];
    const modelBlock = match[0];
    const schemaMatch = modelBlock.match(schemaDirectiveRegex);
    const schemaName = schemaMatch ? schemaMatch[1] : 'public'; // Assuming 'public' for models without @@schema

    modelSchemaMap[modelName] = schemaName;
  }

  return modelSchemaMap;
};
// ...
const teardown = async () => {
  const fs = require('fs')

  const quoteStyle = await getQuoteStyle()
  const schemaMap = extractModelSchemas(dbSchemaPath) // <-- Get model-to-schema mapping

  for (const modelName of teardownOrder) {
    try {
      // Add schema prefix
      await getProjectDb().$executeRawUnsafe(
        `DELETE FROM ${quoteStyle}${schemaMap[modelName]}${quoteStyle}.${quoteStyle}${modelName}${quoteStyle}`
      )
    } catch (e) {
// ...

Now my RedwoodJs project fully support Multi-Schema Prisma.

How do we reproduce the bug?

public repo

Prerequisites

  • PostgreSQL
  • A configured .env file with a valid DATABASE_URL and TEST_DATABASE_URL pointing to your PostgreSQL instance.

Installation

git clone https://github.com/zaiyou12/redwood-multi-schema
cd redwood-multi-schema
yarn install

Reproducing the error

yarn rw prisma migrate dev

yarn rw test api posts

You should encounter the following error:

PrismaClientKnownRequestError:
Invalid `prisma.$executeRawUnsafe()` invocation:

Raw query failed. Code: `42P01`. Message: `relation "Post" does not exist`

What's your environment? (If it applies)

Mac, Postgres on Docker container

Are you interested in working on this?

  • I'm interested in working on this
@zaiyou12 zaiyou12 added the bug/needs-info More information is needed for reproduction label Feb 14, 2024
@jtoar
Copy link
Contributor

jtoar commented Feb 14, 2024

Hey @zaiyou12 thanks for the awesome write up! Indeed we never designed for Prisma's multiSchema feature, but I saw another issue with it come up earlier today so it may be something we should consider supporting.

I couldn't quite get your reproduction working out of the box. After git cloning, yarn installing, and setting up DATABASE_URL and TEST_DATABASE_URL env vars, when I run yarn rw prisma migrate dev, it asks me for a migration:

redwood-multi-schema % yarn rw prisma migrate dev

Running Prisma CLI...
$ yarn prisma migrate dev --schema /Users/dom/projects/redwood/redwood-multi-schema/api/db/schema.prisma

Prisma schema loaded from api/db/schema.prisma
Datasource "db": PostgreSQL database "triage", schemas "cms, public" at "localhost:5432"

PostgreSQL database triage created at localhost:5432

Applying migration `20240214133646_initial_schema`
Applying migration `20240214134022_change_to_cms_schema`

The following migration(s) have been applied:

migrations/
  └─ 20240214133646_initial_schema/
    └─ migration.sql
  └─ 20240214134022_change_to_cms_schema/
    └─ migration.sql
? Enter a name for the new migration: › 

One thing I could try that I didn't yet is yarn rw prisma db push instead of migrate dev.

These days we're trying to focus on RSCs but if you're happy to make a PR for this feature (you seem quite familiar with the code already!), I can try to find some time to review it, I may just not be swift.

@jtoar jtoar added topic/prisma and removed bug/needs-info More information is needed for reproduction labels Feb 14, 2024
@zaiyou12
Copy link
Contributor Author

Hello @jtoar ,

Thank you so much for taking the time to review the issue. I'm glad to contribute and help improve support for Prisma's multiSchema feature in RedwoodJS.

I've identified and corrected a mistake in the schema.prisma file and have updated the repository accordingly. If you're still encountering issues or if the reproduction steps weren't clear, I'd be happy to provide more detailed instructions or assistance.

Regarding the contribution of a PR for this feature, I fully understand the team's current focus and priorities and I am also very much looking forward to the next version. Although I have not personally encountered any issues thus far, I am closely monitoring the related discussions on GitHub. If there are any specific concerns or scenarios you believe should be addressed, I welcome your insights.

Once again, thank you for your time and consideration. I am working on preparing the enhancements for review and will submit the PR as soon as it is ready.

@dthyresson
Copy link
Contributor

Hi @zaiyou12 and thanks also for brining this up.

I was curious if you had tried the reset test strategy as noted in https://redwoodjs.com/docs/testing#the-test-database

image

Since this will use the command yarn rw prisma migrate reset instead it might create that other schema and reset vs just trying to remove data from specific tables.

@zaiyou12
Copy link
Contributor Author

Hi @dthyresson,

Thank you very much for your suggestion and for taking the time to engage with this issue. I followed your advice and tried implementing the reset test strategy as described in the RedwoodJs docs, hoping it would address the multi-schema support challenge in the Jest setup.

I updated my issue-reproduction repository to reflect this approach. (d15a999) Unfortunately, after applying the reset test strategy and running my tests again, the issue still persists. Here's a brief overview of the steps I followed after your recommendation:

  1. Add TEST_DATABASE_STRATEGY=reset in .env file
  2. Executed yarn rw prisma migrate reset to reset the database and apply migrations, aiming to correctly set up the additional schema.
  3. Reran the test to check if the issue was resolved. yarn rw test api posts

Despite these efforts, I'm still facing challenges with multi-schema support in the Jest setup for my Prisma project. The tests fail to recognize or correctly interact with tables in the secondary schema, leading to the same errors as before.

Could there be another aspect of Prisma's multi-schema handling or Jest configuration that we're overlooking? I'm keen on any further insights or alternative suggestions you or anyone else might have to help overcome this hurdle.

Thank you once again for your support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants