Add tests for UsersManager (#1149)

Context

HomeDBManager lacks of direct tests, which makes hard to make rework or refactorations.
Proposed solution

Specifically here, I introduce tests which call exposed UsersManager methods directly and check their result.

Also:

    I removed updateUserName which seems to me useless (updateUser does the same work)
    Taking a look at the getUserByLogin methods, it appears that Typescirpt infers it returns a Promise<User|null> while in no case it may resolve a nullish value, therefore I have forced to return a Promise<User> and have changed the call sites to reflect the change.

Related issues

I make this change for then working on #870

app/server/lib/TestLogin.ts

@@ -25,10 +25,8 @@ export async function getTestLoginSystem(): Promise<GristLoginSystem> {
           if (process.env.TEST_SUPPORT_API_KEY) {
             const dbManager = gristServer.getHomeDBManager();
             const user = await dbManager.getUserByLogin(SUPPORT_EMAIL);
-            if (user) {
-              user.apiKey = process.env.TEST_SUPPORT_API_KEY;
-              await user.save();
-            }
+            user.apiKey = process.env.TEST_SUPPORT_API_KEY;
+            await user.save();
           }
           return "test-login";
         },

app/server/lib/dbUtils.ts

@@ -45,38 +45,53 @@ export async function updateDb(connection?: Connection) {
   await synchronizeProducts(connection, true);
 }
 
+export function getConnectionName() {
+  return process.env.TYPEORM_NAME || 'default';
+}
+
 /**
  * Get a connection to db if one exists, or create one. Serialized to
  * avoid duplication.
  */
 const connectionMutex = new Mutex();
+
+async function buildConnection(overrideConf?: Partial<DataSourceOptions>) {
+  const settings = getTypeORMSettings(overrideConf);
+  const connection = await createConnection(settings);
+  // When using Sqlite, set a busy timeout of 3s to tolerate a little
+  // interference from connections made by tests. Logging doesn't show
+  // any particularly slow queries, but bad luck is possible.
+  // This doesn't affect when Postgres is in use. It also doesn't have
+  // any impact when there is a single connection to the db, as is the
+  // case when Grist is run as a single process.
+  if (connection.driver.options.type === 'sqlite') {
+    await connection.query('PRAGMA busy_timeout = 3000');
+  }
+  return connection;
+}
+
 export async function getOrCreateConnection(): Promise<Connection> {
-  return connectionMutex.runExclusive(async() => {
+  return connectionMutex.runExclusive(async () => {
     try {
       // If multiple servers are started within the same process, we
       // share the database connection.  This saves locking trouble
       // with Sqlite.
-      const connection = getConnection();
-      return connection;
+      return getConnection(getConnectionName());
     } catch (e) {
       if (!String(e).match(/ConnectionNotFoundError/)) {
         throw e;
       }
-      const connection = await createConnection(getTypeORMSettings());
-      // When using Sqlite, set a busy timeout of 3s to tolerate a little
-      // interference from connections made by tests. Logging doesn't show
-      // any particularly slow queries, but bad luck is possible.
-      // This doesn't affect when Postgres is in use. It also doesn't have
-      // any impact when there is a single connection to the db, as is the
-      // case when Grist is run as a single process.
-      if (connection.driver.options.type === 'sqlite') {
-        await connection.query('PRAGMA busy_timeout = 3000');
-      }
-      return connection;
+      return buildConnection();
     }
   });
 }
 
+export async function createNewConnection(overrideConf?: Partial<DataSourceOptions>): Promise<Connection> {
+  return connectionMutex.runExclusive(async () => {
+    return buildConnection(overrideConf);
+  });
+}
+
 export async function runMigrations(connection: Connection) {
   // on SQLite, migrations fail if we don't temporarily disable foreign key
   // constraint checking.  This is because for sqlite typeorm copies each
@@ -103,7 +118,7 @@ export async function undoLastMigration(connection: Connection) {
 // Replace the old janky ormconfig.js file, which was always a source of
 // pain to use since it wasn't properly integrated into the typescript
 // project.
-export function getTypeORMSettings(): DataSourceOptions {
+export function getTypeORMSettings(overrideConf?: Partial<DataSourceOptions>): DataSourceOptions {
   // If we have a redis server available, tell typeorm.  Then any queries built with
   // .cache() called on them will be cached via redis.
   // We use a separate environment variable for the moment so that we don't have to
@@ -120,7 +135,7 @@ export function getTypeORMSettings(): DataSourceOptions {
   } : undefined;
 
   return {
-    "name": process.env.TYPEORM_NAME || "default",
+    "name": getConnectionName(),
     "type": (process.env.TYPEORM_TYPE as any) || "sqlite",  // officially, TYPEORM_CONNECTION -
                                                    // but if we use that, this file will never
                                                    // be read, and we can't configure
@@ -144,5 +159,6 @@ export function getTypeORMSettings(): DataSourceOptions {
     ],
     ...JSON.parse(process.env.TYPEORM_EXTRA || "{}"),
     ...cache,
+    ...overrideConf,
   };
 }