Feature/add postgres schema support

[shubham-padia]

What does this PR do?

Fixes #1818

Changes + decisions:

First commit:

• This commit adds postgres schema support to the app logic. The dev environment uses synchronize function to create tables, and does not run the explicit migrations. We will add schema support for production in the next commit.

Second commit:

• Unlike dev environment, schema will not be created if it does not exist and instead will throw an error. The dev should also make sure that they have sufficient usage and create priviliges on the schema.
• The env variables in .env file will not be picked up by the migration script. Please export them explicitly.
• The migration script inside migration/sql was copied to the postgres directory to accomodate the postgres schema. That migration will now not run for both mssql and postgres.
• You might notice that we set the schema in the dataSource from the env variable and we still modify the SQL strings in migrations file to include schema name. This is because queryRunner when running raw sql commands runs the commands directly without modification. But we still need to set the schema because TypeORM does a select operation on _jackson_migrations table and the schema name is needed in the dataSource for that.

Type of change

• New feature (non-breaking change which adds functionality)

How should this be tested?

I have added tests for the dev environment to cover the first commit. For the second commit, I couldn't find any automated tests, so please test this manually

• create a new schema in your postgres db. Ensure that they have USAGE and CREATE priviliges. A likely error without that would be: link
• export DB_URL and POSTGRES_SCHEMA manually so that they are picked up as env variables.
• run migrations for postgres.

Question:

• .env.example explains what POSTGRES_SCHEMA does. Should it be added to the BoxyHQ online docs. If so, where would be the best place for that?

Checklist:

• My code follows the style guidelines of this project
• I have performed a self-review of my own code and corrected any misspellings
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

[shubham-padia]

We needed a different DB object for the non default schema, that is why this condition. Previously, the previous db would have been overwritten by the new one.

[shubham-padia]

Not the cleanest solution, but since we want to expose a limited number of methods in DatabaseDriver, we needed to access private members of the class to test our changes

[shubham-padia]

None of the test suites has ran, I think I'll need someone's help to trigger them!

[deepakprabhakara]

@shubham-padia Should not modify existing migration scripts, that will break all existing migrations. We'll need separate migrations scripts for custom schema.

[shubham-padia]

@deepakprabhakara

@shubham-padia Should not modify existing migration scripts, that will break all existing migrations. We'll need separate migrations scripts for custom schema.

1. Would agree with that if we were changing the data structure anywhere. The second commit on this PR just adds the schema prefix to each table name without modifying the migration name. TypeORM is tracking these migrations by name in _jackson_migrations , and since none of those names have changed, the migrations would not be ran again for existing installations. In regards to the new file that you see in the second commit, as mentioned in the commit message, it is copied from sql/1692817789888-namespace.ts and the migration name has been kept the same. So that also won't run again. Not sure if generating new migrations would be the best idea if there is no data structure change

2. I have another question though, when someone installs jackson in production, are we using synchronize or these migration scripts to create the tables and get their database up to date? I assumed the migration scripts are ran since TypeORM discourages use of synchronize in production, but If synchronize is ran for all new installations without running the migrations, then we might not need changes to migrations :)

3. The e2e tests are failing with the following error, not sure if its a CI setup failure or some change from my PR. I believe NEXTAUTH_SECRET is not set on the GitHub Actions side. Can you help me check whether its a problem with the CI setup or not since my PR does not change anything related to secrets, thanks :)

Error: [WebServer] [next-auth][error][NO_SECRET] 
https://next-auth.js.org/errors#no_secret Please define a `secret` in production. MissingSecret [MissingSecretError]: Please define a `secret` in production.
    at assertConfig (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next-auth/core/lib/assert.js:42:12)
    at AuthHandler (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next-auth/core/index.js:77:52)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async NextAuthApiHandler (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next-auth/next/index.js:22:19)
    at async NextAuth._args$ (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next-auth/next/index.js:[108](https://github.com/boxyhq/jackson/actions/runs/8582034943/job/23522024213?pr=2540#step:17:109):14)
    at async K (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next/dist/compiled/next-server/pages-api.runtime.prod.js:20:16545)
    at async U.render (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next/dist/compiled/next-server/pages-api.runtime.prod.js:20:16981)
    at async NextNodeServer.runApi (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next/dist/server/next-server.js:554:9)
    at async NextNodeServer.handleCatchallRenderRequest (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next/dist/server/next-server.js:266:37)
    at async NextNodeServer.handleRequestImpl (/home/runner/work/jackson/jackson/.next/standalone/node_modules/next/dist/server/base-server.js:791:17) {
  code: 'NO_SECRET'
}

[deepakprabhakara]

@shubham-padia I believe TypeORM uses a checksum on the migrations files so modifying it will result in the migration failing. Would be good to verify this by running the old migration files and then running yours again to see if it works.

Github Actions will not expose our secrets to this PR (because it's external). We'll check if we can override this.

[shubham-padia]

@deepakprabhakara I checked the source code and this is how TypeOrm loads executed migrations, it makes a query to the migrations table which in our case is _jackson_migrations. This is how pending migrations are calculated, it does not use a checksum and relies on the list of executed migrations by the migration name as shown in the previous link. The name in that table is the same name that we give to the migrations, see screenshot below

Screenshot of query on _jackson_migrations

I also manually verified it using the following steps:

• Checkout the main branch.
• npm run db:migration:run:postgres
• Checkout this PR's branch
• npm run db:migration:run:postgres.

In the last step, no migrations were ran. You can check my terminal output below.

Terminal output

 shubhampadia@192  ~/jackson/npm   main  export DB_URL=postgres://postgres:postgres@localhost:5432/test_jackson
 shubhampadia@192  ~/jackson/npm   main  npm run db:migration:run:postgres

> @boxyhq/[email protected] db:migration:run:postgres
> ts-node --transpile-only ./node_modules/typeorm/cli.js migration:run -d typeorm.ts

query: SELECT * FROM current_schema()
query: SELECT version();
query: SELECT * FROM "information_schema"."tables" WHERE "table_schema" = 'public' AND "table_name" = '_jackson_migrations'
query: CREATE TABLE "_jackson_migrations" ("id" SERIAL NOT NULL, "timestamp" bigint NOT NULL, "name" character varying NOT NULL, CONSTRAINT "PK_ade6b12009d11db8546d10a0383" PRIMARY KEY ("id"))
query: SELECT * FROM "_jackson_migrations" "_jackson_migrations" ORDER BY "id" DESC
0 migrations are already loaded in the database.
4 migrations were found in the source code.
4 migrations are new migrations must be executed.
query: START TRANSACTION
query: CREATE TABLE "jackson_store" ("key" character varying(1500) NOT NULL, "value" text NOT NULL, "iv" character varying(64), "tag" character varying(64), CONSTRAINT "PK_87b6fc1475fbd1228d2f53c6f4a" PRIMARY KEY ("key"))
query: CREATE TABLE "jackson_index" ("id" SERIAL NOT NULL, "key" character varying(1500) NOT NULL, "storeKey" character varying(1500) NOT NULL, CONSTRAINT "PK_a95aa83f01e3c73e126856b7820" PRIMARY KEY ("id"))
query: CREATE INDEX "_jackson_index_key" ON "jackson_index" ("key")
query: CREATE INDEX "_jackson_index_key_store" ON "jackson_index" ("key", "storeKey")
query: CREATE TABLE "jackson_ttl" ("key" character varying(1500) NOT NULL, "expiresAt" bigint NOT NULL, CONSTRAINT "PK_7c9bcdfb4d82e873e19935ec806" PRIMARY KEY ("key"))
query: CREATE INDEX "_jackson_ttl_expires_at" ON "jackson_ttl" ("expiresAt")
query: ALTER TABLE "jackson_index" ADD CONSTRAINT "FK_937b040fb2592b4671cbde09e83" FOREIGN KEY ("storeKey") REFERENCES "jackson_store"("key") ON DELETE CASCADE ON UPDATE NO ACTION
query: INSERT INTO "_jackson_migrations"("timestamp", "name") VALUES ($1, $2) -- PARAMETERS: [1640877103193,"Initial1640877103193"]
Migration Initial1640877103193 has been  executed successfully.
query: ALTER TABLE "jackson_store" ADD "createdAt" TIMESTAMP NOT NULL DEFAULT now()
query: ALTER TABLE "jackson_store" ADD "modifiedAt" TIMESTAMP
query: INSERT INTO "_jackson_migrations"("timestamp", "name") VALUES ($1, $2) -- PARAMETERS: [1644332647279,"createdAt1644332647279"]
Migration createdAt1644332647279 has been  executed successfully.
query: ALTER TABLE "jackson_store" ADD "namespace" character varying(64)
query: CREATE INDEX "_jackson_store_namespace" ON "jackson_store" ("namespace")
query: INSERT INTO "_jackson_migrations"("timestamp", "name") VALUES ($1, $2) -- PARAMETERS: [1692767993709,"PgNamespace1692767993709"]
Migration PgNamespace1692767993709 has been  executed successfully.
query: select jackson.key from jackson_store jackson
query: INSERT INTO "_jackson_migrations"("timestamp", "name") VALUES ($1, $2) -- PARAMETERS: [1692817789888,"namespace1692817789888"]
Migration namespace1692817789888 has been  executed successfully.
query: COMMIT
 shubhampadia@192  ~/jackson/npm   main  git checkout feature/add-postgres-schema-support
Switched to branch 'feature/add-postgres-schema-support'
 shubhampadia@192  ~/jackson/npm   feature/add-postgres-schema-support  npm run build

> @boxyhq/[email protected] build
> tsc -p tsconfig.build.json

 shubhampadia@192  ~/jackson/npm   feature/add-postgres-schema-support  npm run db:migration:run:postgres

> @boxyhq/[email protected] db:migration:run:postgres
> ts-node --transpile-only ./node_modules/typeorm/cli.js migration:run -d typeorm.ts

query: SELECT * FROM current_schema()
query: SELECT version();
query: SELECT * FROM "information_schema"."tables" WHERE "table_schema" = 'public' AND "table_name" = '_jackson_migrations'
query: SELECT * FROM "public"."_jackson_migrations" "_jackson_migrations" ORDER BY "id" DESC
No migrations are pending

[shubham-padia]

@deepakprabhakara just checking in on the review status for this PR :) !

[niwsa]

@shubham-padia We will get around to this shortly. Need to test it out fully before we push it through.

[sbking]

@niwsa Any update here?