Cvmcosta / ltijs-sequelize

Ltijs Sequelize Database Plugin
Apache License 2.0
5 stars 9 forks source link

Migrations fail if using custom Schemas #12

Closed PanopticaRising closed 3 years ago

PanopticaRising commented 3 years ago

In the architecture I'm working on, I'm segregating the LTIjs database into a separate Postgres Schema by providing options to the LTIjs-sequelize constructor:

const scopedLTIDB = new LTIDatabase(name, user, password, {
    host: host,
    dialect: 'postgresql',
    schema: configurations.lti.schema,
    logging: logging,
});

lti.setup(configurations.lti.secret,
    {
        plugin: scopedLTIDB,
    }, {...etc})

This causes the following error on a migration from v2.3.3 to ^2.4.0.

// Correct Schema prefix.
Executing (default): SELECT "name" FROM "LTI"."SequelizeMeta" AS "SequelizeMeta" ORDER BY "SequelizeMeta"."name" ASC;

{ event: 'migrating', name: '00_addAuthorizationServer.js' }

// Invalid Schema prefix.
Executing (default): ALTER TABLE "public"."platforms" ADD COLUMN "authorizationServer" VARCHAR(255) DEFAULT NULL;
Error during deployment:  DatabaseError [SequelizeDatabaseError]: relation "public.platforms" does not exist

This seems to be an open bug with Sequelize (11522, 11742).

I was able to find a workaround by altering the definition of the migration for my use case:

{
    tableName: 'platforms',
    schema: 'LTI',
  }

But this obviously isn't great, and I'm not sure if there's a way to pass the schema down from the constructor. I tried a more generic approach by adding a schema option to Umzug's SequelizeStorage object, but that didn't work, (it probably only applies to the SequelizeMeta table, and uses the getQueryInterface for the migration, which would have the same problem as a raw migration).

    const umzug = new Umzug({
      migrations: {
        glob: path.join(__dirname, 'migrations') + '/*.js'
      },
      context: sequelize.getQueryInterface(),
      storage: new SequelizeStorage({
        sequelize,
        schema: sequelize.options.schema
      }),
      logger: console
    });

Ultimately, not sure if anything else can be done since this is a Postgres-specific feature, but I'm documenting it to save anyone else a trip down the rabbit hole.

Unrelated:

Also, this probably isn't an issue since this migration only has one modification, but we may want to wrap it for consistency with future migrations in a Transaction so that failures can be rolled back together, ie:

await queryInterface.sequelize.transaction(async () => {
  await queryInterface.addColumn(
PanopticaRising commented 3 years ago

I'm closing this because it's not really a bug with this library, and I haven't found a clear workaround.