feat: persisting the database

[artursudnik]

This PR adds:

• support for both SQLite persisted in file and Postgres.
• automating migration files creation and execution for both DB engines
• configuration options to adjust TypeORM settings adequate for running VC-API service in development or production
• keeps default settings so that already deployed instances need no adjustments to keep running with in-memory database
• adds docker-compose.dev.yaml to start developing locally easier

[JGiter]

This variable is also set in .env. Can we skip it here?

[artursudnik]

The idea is to have this change backward compatible with previous deployments of the SSI. Now, they use an in-memory SQLite database. This line ensures that if a container is started without DB_TYPE set, it executes as before.

[JGiter]

Same as above - ideally to configure by .env and have single migration:generate

[artursudnik]

Yes, this was the first approach, but after exercising the creation of migrations a few times, I created two migration scripts in the package.json because when generating migrations, it is necessary to execute both for Postgres and SQLite one after another. Editing .env every time would be really annoying and prone to mistakes. :)

[JGiter]

it is necessary to execute both for Postgres and SQLite

Could you give an example for such scenario?

[artursudnik]

Postgres and SQLite are different DB engines, so migration files are different for them. If we want SSI to be able to use any of them, we need to provide migrations for both of them that differ. So, when DB schema is extended during development, it is necessary to generate a new migration file for Postgres and SQLite. It is not possible to execute Postgres migration on SQLite SQLite migration on Postgres.

[JGiter]

We can remove this now, since sqlite config is returned by typeOrmConfigFactory

[artursudnik]

It is still used in tests and only there as they are executed with an in-memory database.

[JGiter]

Hmm, why tests can't use typeOrmConfigFactory(config of sqlite datasource) ?

[artursudnik]

It was just simpler for me to keep this as a wrapper than mocking ConfigService and injecting it in tests. Do you think it is worth the effort to do this?

[artursudnik]

it would look like this, what do you think?

        TypeOrmModule.forRoot(
          typeOrmConfigFactory({
            get: (key): Record<string, unknown> => {
              return { DB_TYPE: DB_TYPES.SQLITE_IN_MEMORY }[key];
            }
          } as unknown as ConfigService)
        ),

instead of this:

typeOrmInMemoryModuleFactory(),

[JGiter]

why another clause of .when? Can we add is in previouse when.switch?

[artursudnik]

I needed to use this hack as setting common attributes for is: Joi.required().valid(DB_TYPES.SQLITE, DB_TYPES.POSTGRES) in the switch is not possible. It triggers some inconsistent behavior of Joi.

Maybe, repeating this twice would be more readable? What do you think?

DB_DROP_ON_START: Joi.boolean().default(false),
DB_SYNCHRONIZE: Joi.boolean().default(false),
DB_RUN_MIGRATIONS: Joi.boolean().default(true)

[artursudnik]

So, it would look like this, but we would have repeated code:

.when('.DB_TYPE', {
    switch: [
      {
        is: Joi.required().valid(DB_TYPES.SQLITE),
        then: {
          SQLITE_FILE: Joi.string().required(),
          DB_DROP_ON_START: Joi.boolean().default(false),
          DB_SYNCHRONIZE: Joi.boolean().default(false),
          DB_RUN_MIGRATIONS: Joi.boolean().default(true)
        }
      },
      {
        is: Joi.required().valid(DB_TYPES.POSTGRES),
        then: {
          POSTGRES_DB_HOST: Joi.string().hostname().required(),
          POSTGRES_DB_PORT: Joi.number().port().required(),
          POSTGRES_DB_USER: Joi.string().required(),
          POSTGRES_DB_PASSWORD: Joi.string().required(),
          POSTGRES_DB_NAME: Joi.string().required(),
          DB_DROP_ON_START: Joi.boolean().default(false),
          DB_SYNCHRONIZE: Joi.boolean().default(false),
          DB_RUN_MIGRATIONS: Joi.boolean().default(true)
        }
      }
    ]
  })

[JGiter]

If these variables independent from DB_TYPE can we validate them outside of when(DB_TYPE?

[artursudnik]

They are dependent on the DB_TYPE. They are required for Postgres and SQLite, but not for in-memory SQLite

[JGiter]

Then may be add .with(DB_TYPE, DP_DROP_ON_START,..)?

[artursudnik]

But .with would require them to be defined and optional first, but in the case of in-memory SQLite, they are ignored. I think this may be confusing for someone who will make interpretations based on this schema.

[artursudnik]

anyway .with here would require keys to exist for DB_TYPE defined, not set to a given value

[jrhender]

Looks good, some minor comments/questions

[jrhender]

@artursudnik why is this step necessary?

[artursudnik]

@jrhender Because during development we usually execute the service with DB_SYNC_SCHEMA_ON_START=true and before generating a new migration we need to recreate the schema to be in sync with existing migrations only. Then, TypeORM is able to calculate the delta between modified entities and the schema defined before.

[jrhender]

Thanks for the explanation, I know that isn't the first time you've explained it to me 😅
Maybe we can add the explanation to the documentation.

Suggested change

	1. [drop db schemas](#dropping-schemas) for both Postgres and SQLite
	1. [drop db schemas](#dropping-schemas) for both Postgres and SQLite. This is useful because, if `DB_SYNC_SCHEMA_ON_START=true` is used during development, then the db schemas needs to be in sync with existing migrations only prior to generating new ones.

[jrhender]

By "validate migration files", did you mean that the files should be manually checked to make sure that they look correct?

[artursudnik]

@jrhender Yes, they need to be at least reformatted because the formatting of generated files differs from what is set for the repo. I would also execute all migrations together with new ones and then execute tests.

[jrhender]

Thanks. By executing tests, I guess you meant manual tests of the API?

Suggested change

	6. validate migration files
	6. validate migration files. This should include executing all migrations (including new ones) and then executing manually tests of the API.

[artursudnik]

Yes, I am adding a suggestion to do this for both SQLite and Postgres