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

feat(data-store): initialize DB using migrations #679

Merged
merged 4 commits into from
Aug 31, 2021

Conversation

mirceanis
Copy link
Member

@mirceanis mirceanis commented Aug 25, 2021

fixes #676

BREAKING CHANGES

The @veramo/data-store package relies on typeorm as a database abstraction.
Typeorm has a connection flag synchonize which bootstraps the database along with schema and relations based on a set of Entities (annotated typescript classes).
This is very handy for fast development iterations but it is not recommended for production use because there is too much ambiguity possible when the Entities change, and there is a risk of data loss.
The recommended way to do things is to use the migrations mechanism. It allows you to migrate to new database schemas when necessary, and even customize the database to your own needs.

Going forward, this is the mechanism we will be recommending for connections.

To adopt this, there are some small changes needed for your agent configuration:

new @veramo/cli configuration:

If you're already running an agent using the @veramo/cli package, please update the dbConnection section of your configuration like so:

# Data base
dbConnection:
  $require: typeorm?t=function#createConnection
  $args:
    - type: sqlite
      database:
        $ref: /constants/databaseFile
      synchronize: false    # set this to false
      migrationsRun: true   # set this to true
      migrations:           # and import the default migrations from veramo
        $require: '@veramo/data-store?t=object#migrations' 
      logging: false
      entities:
        $require: '@veramo/data-store?t=object#Entities'

If you haven't edited your configuration before, you can simply create a new config file with the veramo config create command.

new JS/TS configuration:

import { Entities, migrations, /*...*/ } from '../packages/data-store/src'
import { createConnection } from 'typeorm'
// ...
const dbConnection = createConnection({
   // ...
    synchronize: false,     // set this to false
    migrations: migrations, // import the default migrations from veramo
    migrationsRun: true,    // enable this flag
    logging: false,
    entities: Entities,
  })

// the agent configuration doesn't actually change, only the `dbConnection` from above.
agent = createAgent<>({
  plugins: [
    // ... use the dbConnection from above with the plugins that require it
    new DataStore(dbConnection),
    new DataStoreORM(dbConnection),
  ]
})

PR Tasks

  • create initial migrations using querryRunner API
    • align cascade options
    • align nullable: true with respective Entity properties
  • add test for DB initialized using migrations
    • decouple test data. Test suites should be able to run in isolation.
  • add smoke tests using migrated data ( see initial.migration.test.ts)
    • get key
    • get identifier with keys and services
    • get credential
    • get credential by claim
    • get presentation
    • get message with credentials and presentation
  • run tests using different DB drivers (sqlite & postgres)
  • check for data loss if migrations are NOT used
    • app will fail to start because of broken FOREIGN KEY constraints if you try to start without migrations and with synchronize: true

@codecov
Copy link

codecov bot commented Aug 25, 2021

Codecov Report

Merging #679 (0c8dcc5) into next (88264db) will increase coverage by 11.91%.
The diff coverage is 76.13%.

@@             Coverage Diff             @@
##             next     #679       +/-   ##
===========================================
+ Coverage   67.58%   79.49%   +11.91%     
===========================================
  Files          62       80       +18     
  Lines        1530     2424      +894     
  Branches      247      418      +171     
===========================================
+ Hits         1034     1927      +893     
- Misses        400      495       +95     
+ Partials       96        2       -94     

@mirceanis mirceanis force-pushed the 676-initialize-db-using-migrations branch from c765a3f to 48f3926 Compare August 26, 2021 10:30
@mirceanis mirceanis force-pushed the 676-initialize-db-using-migrations branch from 48f3926 to b54ce40 Compare August 26, 2021 15:07
@mirceanis mirceanis force-pushed the 676-initialize-db-using-migrations branch from 6047743 to f78c160 Compare August 31, 2021 10:20
@mirceanis mirceanis force-pushed the 676-initialize-db-using-migrations branch from f78c160 to 0c8dcc5 Compare August 31, 2021 10:36
@mirceanis mirceanis marked this pull request as ready for review August 31, 2021 11:14
Copy link
Contributor

@simonas-notcat simonas-notcat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Awesome work @mirceanis !

@mirceanis mirceanis merged commit 41f6240 into next Aug 31, 2021
@mirceanis mirceanis deleted the 676-initialize-db-using-migrations branch August 31, 2021 12:11
@mirceanis mirceanis mentioned this pull request Sep 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

change to true DB migration (away from synchronize that breaks in production)
2 participants