From e25c8ac6419c4cedd22d6815242e131f97dbbac3 Mon Sep 17 00:00:00 2001
From: AkiraFukushima <h3.poteto@gmail.com>
Date: Thu, 11 Oct 2018 23:12:22 +0900
Subject: [PATCH] refs #651 Reoreder accounts and fix order method

---
 src/main/account.js                       | 194 ++++++++++++++++------
 src/main/auth.js                          |   9 +-
 src/main/index.js                         |  50 +++---
 src/renderer/store/Preferences/Account.js |   2 +
 4 files changed, 170 insertions(+), 85 deletions(-)

diff --git a/src/main/account.js b/src/main/account.js
index 837221c3..03209f8d 100644
--- a/src/main/account.js
+++ b/src/main/account.js
@@ -6,6 +6,56 @@ export default class Account {
     this.db = db
   }
 
+  async initialize () {
+    await this.updateUnique()
+    await this.cleanup()
+    await this.reorder()
+  }
+
+  updateUnique () {
+    return new Promise((resolve, reject) => {
+      // At first, remove old index.
+      this.db.removeIndex('order', (err) => {
+        if (err) reject(err)
+        // Add unique index.
+        this.db.ensureIndex({ fieldName: 'order', unique: true, sparse: true }, (err) => {
+          if (err) reject(err)
+          resolve(null)
+        })
+      })
+    })
+  }
+
+  /**
+   * Reorder accounts, because sometimes the order of accounts is duplicated.
+   */
+  async reorder () {
+    const accounts = await this.listAllAccounts()
+    await Promise.all(accounts.map(async (account, index) => {
+      const update = await this.updateAccount(account._id, Object.assign(account, { order: index + 1 }))
+      return update
+    }))
+    const ordered = await this.listAllAccounts()
+    return ordered
+  }
+
+  /**
+   * Check order of all accounts, and fix if order is negative value or over the length.
+   */
+  async cleanup () {
+    const accounts = await this.listAccounts()
+    if (accounts.length < 1) {
+      return accounts.length
+    }
+    if (accounts[0].order < 1 || accounts[accounts.length - 1].order > accounts.length) {
+      await Promise.all(accounts.map(async (element, index) => {
+        const update = await this.updateAccount(element._id, Object.assign(element, { order: index + 1 }))
+        return update
+      }))
+    }
+    return null
+  }
+
   insertAccount (obj) {
     return new Promise((resolve, reject) => {
       this.db.insert(obj, (err, doc) => {
@@ -16,6 +66,22 @@ export default class Account {
     })
   }
 
+  /**
+   * List up all accounts either authenticated or not authenticated.
+   */
+  listAllAccounts (order = 1) {
+    return new Promise((resolve, reject) => {
+      this.db.find().sort({ order: order }).exec((err, docs) => {
+        if (err) return reject(err)
+        if (empty(docs)) return reject(new EmptyRecordError('empty'))
+        resolve(docs)
+      })
+    })
+  }
+
+  /**
+   * List up authenticated accounts.
+   */
   listAccounts () {
     return new Promise((resolve, reject) => {
       this.db.find({accessToken: { $ne: '' }}).sort({ order: 1 }).exec((err, docs) => {
@@ -26,13 +92,10 @@ export default class Account {
     })
   }
 
-  countAuthorizedAccounts () {
-    return new Promise((resolve, reject) => {
-      this.db.count({accessToken: { $ne: '' }}, (err, count) => {
-        if (err) return reject(err)
-        resolve(count)
-      })
-    })
+  // Get the last account.
+  async lastAccount () {
+    const accounts = await this.listAllAccounts(-1)
+    return accounts[0]
   }
 
   getAccount (id) {
@@ -62,14 +125,12 @@ export default class Account {
     })
   }
 
-  searchAccounts (obj) {
+  searchAccounts (obj, order = 1) {
     return new Promise((resolve, reject) => {
-      this.db.find(
-        obj,
-        (err, docs) => {
-          if (err) return reject(err)
-          resolve(docs)
-        })
+      this.db.find(obj).sort({ order: order }).exec((err, docs) => {
+        if (err) return reject(err)
+        resolve(docs)
+      })
     })
   }
 
@@ -126,54 +187,81 @@ export default class Account {
   }
 
   async forwardAccount (ac) {
-    if (ac.order <= 1) {
-      return ac.order
+    // Find account which is the previous of the target account.
+    const accounts = await this.searchAccounts({ order: { $lt: ac.order } }, -1)
+      .catch((err) => {
+        console.log(err)
+        return []
+      })
+    if (accounts.length < 1) {
+      return null
     }
-    // Find account which is backwarded
-    const backwarded = await this.searchAccount(
+    const previousAccount = accounts[0]
+    const targetOrder = ac.order
+    const previousOrder = previousAccount.order
+
+    // At first, we need to update the previous account with dummy order.
+    // Because this column is uniqued, so can not update with same order.
+    await this.updateAccount(previousAccount._id, Object.assign(
+      previousAccount,
       {
-        order: ac.order - 1
+        order: -1
       }
-    )
-    await this.updateAccount(backwarded._id, Object.assign(backwarded, { order: (backwarded.order + 1) }))
-    // Forward account order
-    const updated = await this.updateAccount(ac._id, Object.assign(ac, { order: (ac.order - 1) }))
+    ))
+    // Change order of the target account.
+    const updated = await this.updateAccount(ac._id, Object.assign(
+      ac,
+      {
+        order: previousOrder
+      }
+    ))
+    // Update the previous account with right order.
+    await this.updateAccount(previousAccount._id, Object.assign(
+      previousAccount,
+      {
+        order: targetOrder
+      }
+    ))
     return updated
   }
 
   async backwardAccount (ac) {
-    const length = await this.countAuthorizedAccounts()
-    if (ac.order >= length) {
-      return ac.order
-    }
-    // Find account which is forwarded
-    const forwarded = await this.searchAccount(
-      {
-        order: ac.order + 1
-      }
-    )
-    await this.updateAccount(forwarded._id, Object.assign(forwarded, { order: (forwarded.order - 1) }))
-    // Backward account order
-    const updated = await this.updateAccount(ac._id, Object.assign(ac, { order: (ac.order + 1) }))
-    return updated
-  }
-
-  /*
-   * cleanup
-   * Check order of all accounts, and fix if order is negative value or over the length.
-   */
-  async cleanup () {
-    const accounts = await this.listAccounts()
+    // Find account which is the next of the target account.
+    const accounts = await this.searchAccounts({ order: { $gt: ac.order } }, 1)
+      .catch((err) => {
+        console.log(err)
+        return []
+      })
     if (accounts.length < 1) {
-      return accounts.length
+      return null
     }
-    if (accounts[0].order < 1 || accounts[accounts.length - 1].order > accounts.length) {
-      await Promise.all(accounts.map(async (element, index) => {
-        const update = await this.updateAccount(element._id, Object.assign(element, { order: index + 1 }))
-        return update
-      }))
-    }
-    return null
+    const nextAccount = accounts[0]
+    const targetOrder = ac.order
+    const nextOrder = nextAccount.order
+
+    // At first, we need to update the next account with dummy order.
+    // Because this colum is uniqued, so can not update with same order.
+    await this.updateAccount(nextAccount._id, Object.assign(
+      nextAccount,
+      {
+        order: -1
+      }
+    ))
+    // Change order of the target account/
+    const updated = await this.updateAccount(ac._id, Object.assign(
+      ac,
+      {
+        order: nextOrder
+      }
+    ))
+    // Update the next account with right order.
+    await this.updateAccount(nextAccount._id, Object.assign(
+      nextAccount,
+      {
+        order: targetOrder
+      }
+    ))
+    return updated
   }
 
   async refreshAccounts () {
diff --git a/src/main/auth.js b/src/main/auth.js
index b50a11a9..b3d8c7dd 100644
--- a/src/main/auth.js
+++ b/src/main/auth.js
@@ -33,7 +33,12 @@ export default class Authentication {
     this.clientId = res.clientId
     this.clientSecret = res.clientSecret
 
-    const count = await this.db.countAuthorizedAccounts()
+    const order = await this.db.lastAccount()
+      .then(account => account.order + 1)
+      .catch((err) => {
+        console.log(err)
+        return 1
+      })
     const json = {
       baseURL: this.baseURL,
       domain: this.domain,
@@ -43,7 +48,7 @@ export default class Authentication {
       username: '',
       accountId: '',
       avatar: '',
-      order: count + 1
+      order: order
     }
     await this.db.insertAccount(json)
     return res.url
diff --git a/src/main/index.js b/src/main/index.js
index a4aeeac7..fb1dbdc0 100644
--- a/src/main/index.js
+++ b/src/main/index.js
@@ -58,6 +58,10 @@ let accountDB = new Datastore({
   filename: accountDBPath,
   autoload: true
 })
+const accountManager = new Account(accountDB)
+accountManager.initialize()
+  .catch(err => log.error(err))
+
 const hashtagsDBPath = process.env.NODE_ENV === 'production'
   ? userData + '/db/hashtags.db'
   : 'hashtags.db'
@@ -76,9 +80,7 @@ const soundBasePath = process.env.NODE_ENV === 'development'
 
 async function listAccounts () {
   try {
-    const account = new Account(accountDB)
-    await account.cleanup()
-    const accounts = await account.listAccounts()
+    const accounts = await accountManager.listAccounts()
     return accounts
   } catch (err) {
     return []
@@ -240,7 +242,7 @@ app.on('activate', () => {
   }
 })
 
-let auth = new Authentication(new Account(accountDB))
+let auth = new Authentication(accountManager)
 
 ipcMain.on('get-auth-url', (event, domain) => {
   auth.getAuthorizationUrl(domain)
@@ -284,8 +286,7 @@ ipcMain.on('get-social-token', (event, _) => {
 
 // nedb
 ipcMain.on('list-accounts', (event, _) => {
-  const account = new Account(accountDB)
-  account.listAccounts()
+  accountManager.listAccounts()
     .catch((err) => {
       log.error(err)
       event.sender.send('error-list-accounts', err)
@@ -296,8 +297,7 @@ ipcMain.on('list-accounts', (event, _) => {
 })
 
 ipcMain.on('get-local-account', (event, id) => {
-  const account = new Account(accountDB)
-  account.getAccount(id)
+  accountManager.getAccount(id)
     .catch((err) => {
       log.error(err)
       event.sender.send('error-get-local-account', err)
@@ -308,8 +308,7 @@ ipcMain.on('get-local-account', (event, id) => {
 })
 
 ipcMain.on('update-account', (event, acct) => {
-  const account = new Account(accountDB)
-  account.refresh(acct)
+  accountManager.refresh(acct)
     .then((ac) => {
       event.sender.send('response-update-account', ac)
     })
@@ -319,8 +318,7 @@ ipcMain.on('update-account', (event, acct) => {
 })
 
 ipcMain.on('remove-account', (event, id) => {
-  const account = new Account(accountDB)
-  account.removeAccount(id)
+  accountManager.removeAccount(id)
     .then(() => {
       event.sender.send('response-remove-account')
     })
@@ -330,19 +328,18 @@ ipcMain.on('remove-account', (event, id) => {
 })
 
 ipcMain.on('forward-account', (event, acct) => {
-  const account = new Account(accountDB)
-  account.forwardAccount(acct)
+  accountManager.forwardAccount(acct)
     .then(() => {
       event.sender.send('response-forward-account')
     })
     .catch((err) => {
+      log.error(err)
       event.sender.send('error-forward-account', err)
     })
 })
 
 ipcMain.on('backward-account', (event, acct) => {
-  const account = new Account(accountDB)
-  account.backwardAccount(acct)
+  accountManager.backwardAccount(acct)
     .then(() => {
       event.sender.send('response-backward-account')
     })
@@ -352,8 +349,7 @@ ipcMain.on('backward-account', (event, acct) => {
 })
 
 ipcMain.on('refresh-accounts', (event, _) => {
-  const account = new Account(accountDB)
-  account.refreshAccounts()
+  accountManager.refreshAccounts()
     .then((accounts) => {
       event.sender.send('response-refresh-accounts', accounts)
     })
@@ -363,8 +359,7 @@ ipcMain.on('refresh-accounts', (event, _) => {
 })
 
 ipcMain.on('remove-all-accounts', (event, _) => {
-  const account = new Account(accountDB)
-  account.removeAll()
+  accountManager.removeAll()
     .then(() => {
       event.sender.send('response-remove-all-accounts')
     })
@@ -385,8 +380,7 @@ ipcMain.on('reset-badge', () => {
 let userStreaming = null
 
 ipcMain.on('start-user-streaming', (event, ac) => {
-  const account = new Account(accountDB)
-  account.getAccount(ac._id)
+  accountManager.getAccount(ac._id)
     .catch((err) => {
       log.error(err)
       event.sender.send('error-start-user-streaming', err)
@@ -427,8 +421,7 @@ ipcMain.on('stop-user-streaming', (event, _) => {
 let localStreaming = null
 
 ipcMain.on('start-local-streaming', (event, ac) => {
-  const account = new Account(accountDB)
-  account.getAccount(ac._id)
+  accountManager.getAccount(ac._id)
     .catch((err) => {
       log.error(err)
       event.sender.send('error-start-local-streaming', err)
@@ -465,8 +458,7 @@ ipcMain.on('stop-local-streaming', (event, _) => {
 let publicStreaming = null
 
 ipcMain.on('start-public-streaming', (event, ac) => {
-  const account = new Account(accountDB)
-  account.getAccount(ac._id)
+  accountManager.getAccount(ac._id)
     .catch((err) => {
       log.error(err)
       event.sender.send('error-start-public-streaming', err)
@@ -503,8 +495,7 @@ ipcMain.on('stop-public-streaming', (event, _) => {
 let listStreaming = null
 
 ipcMain.on('start-list-streaming', (event, obj) => {
-  const account = new Account(accountDB)
-  account.getAccount(obj.account._id)
+  accountManager.getAccount(obj.account._id)
     .catch((err) => {
       log.error(err)
       event.sender.send('error-start-list-streaming', err)
@@ -541,8 +532,7 @@ ipcMain.on('stop-list-streaming', (event, _) => {
 let tagStreaming = null
 
 ipcMain.on('start-tag-streaming', (event, obj) => {
-  const account = new Account(accountDB)
-  account.getAccount(obj.account._id)
+  accountManager.getAccount(obj.account._id)
     .catch((err) => {
       log.error(err)
       event.sender.send('error-start-tag-streaming', err)
diff --git a/src/renderer/store/Preferences/Account.js b/src/renderer/store/Preferences/Account.js
index dd06e0a3..b7d4540c 100644
--- a/src/renderer/store/Preferences/Account.js
+++ b/src/renderer/store/Preferences/Account.js
@@ -43,6 +43,7 @@ const Account = {
       })
     },
     forwardAccount ({ commit }, account) {
+      console.log(account)
       return new Promise((resolve, reject) => {
         ipcRenderer.send('forward-account', account)
         ipcRenderer.once('error-forward-account', (event, err) => {
@@ -56,6 +57,7 @@ const Account = {
       })
     },
     backwardAccount ({ commit }, account) {
+      console.log(account)
       return new Promise((resolve, reject) => {
         ipcRenderer.send('backward-account', account)
         ipcRenderer.once('error-backward-account', (event, err) => {