issue: ENG-7267
We want to login / register before db starts being set up, so that if the credential are incorrect, and an error is thrown, the db is not left in an inconsistent state
unpickleAndUseCallback is modeled on fetchCallUpdateOlmAccount, but was rid of code that is used for fetching from and saving to the db. I also skipped retries, which only make sense if we operate on the db - they imitate transactions. Instead of fetching and saving account in the db, I remember them in the memory, and return them from verifyUserLoggedInWithoutDB to be saved once the tables get created
Details
Details
Ran my nix script that created a new ks, checked that the admin got correctly registered with identity.
Ran my nix script and provided incorrect credentials - checked that there are no tables in the db
Checked that the accounts saved to the db are the accounts after transformations (logged the accounts at the end of verifyUserLoggedInWithoutDB and checked that the same ones are in the db)
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
Comment Actions
Some nits left but looks good.
| keyserver/src/database/migration-config.js | ||
|---|---|---|
| 851 ↗ | (On Diff #38732) | I would be good to change the name of this function to indicate that it is supposed only to save brand new olm accounts in the database that doesn't already have any. |
| keyserver/src/user/login.js | ||
| 88 ↗ | (On Diff #38732) | This violates the convention: ServerError should contain short and generic message. Look for examples in the code, |
| 94 ↗ | (On Diff #38732) | It looks confusing. I think that using picklingKey directly is more readable. |
| keyserver/src/user/login.js | ||
|---|---|---|
| 88 ↗ | (On Diff #38732) | This is code copied from fetchCallUpdateOlmAccount.... And I don't know how I could fix that, since this is an error thrown from callback, which can throw anything. We could always throw 'unknown_error', would you prefer that? |
| 94 ↗ | (On Diff #38732) | But this will make code more error prone in case we add another field to pickledOlmAccount in the future. This is how we usually do this |